Fix for bug 46280 - RowRecordsAggregate should allow for ContinueRecords after UnkownRecords, and PivotTable records should not get in the RowRecordsAggregate at all

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

9 files changed, 581 insertions, 492 deletions

diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml
index 1580f7ed7c..784f19875e 100644
--- a/src/documentation/content/xdocs/changes.xml
+++ b/src/documentation/content/xdocs/changes.xml
@@ -37,7 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.5-beta5" date="2008-??-??">
-          <action dev="POI-DEVELOPERS" type="fix"><!--remove me--></action>
+           <action dev="POI-DEVELOPERS" type="fix">46280 - Fixed RowRecordsAggregate etc to properly skip PivotTable records</action>
         </release>
         <release version="3.5-beta4" date="2008-11-29">
            <action dev="POI-DEVELOPERS" type="fix">46213 - Fixed FormulaRecordAggregate to gracefully ignore extra StringRecords</action>
diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index a6bc5997fa..59d6b999c8 100644
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -34,7 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.5-beta5" date="2008-??-??">
-          <action dev="POI-DEVELOPERS" type="fix"><!--remove me--></action>
+           <action dev="POI-DEVELOPERS" type="fix">46280 - Fixed RowRecordsAggregate etc to properly skip PivotTable records</action>
         </release>
         <release version="3.5-beta4" date="2008-11-29">
            <action dev="POI-DEVELOPERS" type="fix">46213 - Fixed FormulaRecordAggregate to gracefully ignore extra StringRecords</action>
diff --git a/src/java/org/apache/poi/hssf/model/RecordOrderer.java b/src/java/org/apache/poi/hssf/model/RecordOrderer.java
index 394825917d..6aaa92e887 100644
--- a/src/java/org/apache/poi/hssf/model/RecordOrderer.java
+++ b/src/java/org/apache/poi/hssf/model/RecordOrderer.java
@@ -85,12 +85,12 @@ final class RecordOrderer {
 	 * Adds the specified new record in the correct place in sheet records list
 	 * 
 	 */
-	public static void addNewSheetRecord(List sheetRecords, RecordBase newRecord) {
+	public static void addNewSheetRecord(List<RecordBase> sheetRecords, RecordBase newRecord) {
 		int index = findSheetInsertPos(sheetRecords, newRecord.getClass());
 		sheetRecords.add(index, newRecord);
 	}
 
-	private static int findSheetInsertPos(List records, Class recClass) {
+	private static int findSheetInsertPos(List<RecordBase> records, Class recClass) {
 		if (recClass == DataValidityTable.class) {
 			return findDataValidationTableInsertPos(records);
 		}
@@ -109,7 +109,7 @@ final class RecordOrderer {
 		throw new RuntimeException("Unexpected record class (" + recClass.getName() + ")");
 	}
 
-	private static int getPageBreakRecordInsertPos(List records) {
+	private static int getPageBreakRecordInsertPos(List<RecordBase> records) {
 		int dimensionsIndex = getDimensionsIndex(records);
 		int i = dimensionsIndex-1;
 		while (i > 0) {
@@ -152,7 +152,7 @@ final class RecordOrderer {
 	/**
 	 * Find correct position to add new CFHeader record
 	 */
-	private static int findInsertPosForNewCondFormatTable(List records) {
+	private static int findInsertPosForNewCondFormatTable(List<RecordBase> records) {
 
 		for (int i = records.size() - 2; i >= 0; i--) { // -2 to skip EOF record
 			Object rb = records.get(i);
@@ -175,7 +175,7 @@ final class RecordOrderer {
 		throw new RuntimeException("Did not find Window2 record");
 	}
 
-	private static int findInsertPosForNewMergedRecordTable(List records) {
+	private static int findInsertPosForNewMergedRecordTable(List<RecordBase> records) {
 		for (int i = records.size() - 2; i >= 0; i--) { // -2 to skip EOF record
 			Object rb = records.get(i);
 			if (!(rb instanceof Record)) {
@@ -219,14 +219,14 @@ final class RecordOrderer {
 	 * o RANGEPROTECTION
 	 * + EOF
 	 */
-	private static int findDataValidationTableInsertPos(List records) {
+	private static int findDataValidationTableInsertPos(List<RecordBase> records) {
 		int i = records.size() - 1;
 		if (!(records.get(i) instanceof EOFRecord)) {
 			throw new IllegalStateException("Last sheet record should be EOFRecord");
 		}
 		while (i > 0) {
 			i--;
-			Object rb = records.get(i);
+			RecordBase rb = records.get(i);
 			if (isDVTPriorRecord(rb)) {
 				Record nextRec = (Record) records.get(i + 1);
 				if (!isDVTSubsequentRecord(nextRec.getSid())) {
@@ -245,7 +245,7 @@ final class RecordOrderer {
 	}
 
 
-	private static boolean isDVTPriorRecord(Object rb) {
+	private static boolean isDVTPriorRecord(RecordBase rb) {
 		if (rb instanceof MergedCellsTable || rb instanceof ConditionalFormattingTable) {
 			return true;
 		}
@@ -280,7 +280,7 @@ final class RecordOrderer {
 	/**
 	 * DIMENSIONS record is always present
 	 */
-	private static int getDimensionsIndex(List records) {
+	private static int getDimensionsIndex(List<RecordBase> records) {
 		int nRecs = records.size();
 		for(int i=0; i<nRecs; i++) {
 			if(records.get(i) instanceof DimensionsRecord) {
@@ -291,12 +291,12 @@ final class RecordOrderer {
 		throw new RuntimeException("DimensionsRecord not found");
 	}
 
-	private static int getGutsRecordInsertPos(List records) {
+	private static int getGutsRecordInsertPos(List<RecordBase> records) {
 		int dimensionsIndex = getDimensionsIndex(records);
 		int i = dimensionsIndex-1;
 		while (i > 0) {
 			i--;
-			Object rb = records.get(i);
+			RecordBase rb = records.get(i);
 			if (isGutsPriorRecord(rb)) {
 				return i+1;
 			}
@@ -304,7 +304,7 @@ final class RecordOrderer {
 		throw new RuntimeException("Did not find insert point for GUTS");
 	}
 
-	private static boolean isGutsPriorRecord(Object rb) {
+	private static boolean isGutsPriorRecord(RecordBase rb) {
 		if (rb instanceof Record) {
 			Record record = (Record) rb;
 			switch (record.getSid()) {
@@ -337,6 +337,8 @@ final class RecordOrderer {
 	 */
 	public static boolean isEndOfRowBlock(int sid) {
 		switch(sid) {
+			case UnknownRecord.SXVIEW_00B0:
+				// should have been prefixed with DrawingRecord (0x00EC), but bug 46280 seems to allow this
 			case DrawingRecord.sid:
 			case DrawingSelectionRecord.sid:
 			case ObjRecord.sid:
diff --git a/src/java/org/apache/poi/hssf/model/RowBlocksReader.java b/src/java/org/apache/poi/hssf/model/RowBlocksReader.java
index 020d489417..038304916a 100644
--- a/src/java/org/apache/poi/hssf/model/RowBlocksReader.java
+++ b/src/java/org/apache/poi/hssf/model/RowBlocksReader.java
@@ -45,11 +45,11 @@ public final class RowBlocksReader {
 	 * mergedCellsTable
 	 */
 	public RowBlocksReader(RecordStream rs) {
-		List plainRecords = new ArrayList();
-		List shFrmRecords = new ArrayList();
-		List arrayRecords = new ArrayList();
-		List tableRecords = new ArrayList();
-		List mergeCellRecords = new ArrayList();
+		List<Record> plainRecords = new ArrayList<Record>();
+		List<Record> shFrmRecords = new ArrayList<Record>();
+		List<Record> arrayRecords = new ArrayList<Record>();
+		List<Record> tableRecords = new ArrayList<Record>();
+		List<Record> mergeCellRecords = new ArrayList<Record>();
 
 		while(!RecordOrderer.isEndOfRowBlock(rs.peekNextSid())) {
 			// End of row/cell records for the current sheet
@@ -61,7 +61,7 @@ public final class RowBlocksReader {
 			
 			}
 			Record rec = rs.getNext();
-			List dest;
+			List<Record> dest;
 			switch (rec.getSid()) {
 				case MergeCellsRecord.sid:    dest = mergeCellRecords; break;
 				case SharedFormulaRecord.sid: dest = shFrmRecords;     break;
diff --git a/src/java/org/apache/poi/hssf/record/UnknownRecord.java b/src/java/org/apache/poi/hssf/record/UnknownRecord.java
index 997c35642b..bd16fb6049 100644
--- a/src/java/org/apache/poi/hssf/record/UnknownRecord.java
+++ b/src/java/org/apache/poi/hssf/record/UnknownRecord.java
@@ -39,6 +39,7 @@ public final class UnknownRecord extends StandardRecord {
 	public static final int SHEETPR_0081         = 0x0081;
 	public static final int STANDARDWIDTH_0099   = 0x0099;
 	public static final int SCL_00A0             = 0x00A0;
+	public static final int SXVIEW_00B0          = 0x00B0;
 	public static final int BITMAP_00E9          = 0x00E9;
 	public static final int PHONETICPR_00EF      = 0x00EF;
 	public static final int LABELRANGES_015F     = 0x015F;
@@ -128,13 +129,22 @@ public final class UnknownRecord extends StandardRecord {
 			case 0x0094: return "LHRECORD";
 			case STANDARDWIDTH_0099: return "STANDARDWIDTH";
 			case 0x009D: return "AUTOFILTERINFO";
-			case SCL_00A0: return "SCL";
+			case SCL_00A0:     return "SCL";
 			case 0x00AE: return "SCENMAN";
+			case SXVIEW_00B0: return "SXVIEW"; // (pivot table) View Definition
+			case 0x00B1: return "SXVD";        // (pivot table) View Fields
+			case 0x00B2: return "SXVI";        // (pivot table) View Item
+			case 0x00B4: return "SXIVD";       // (pivot table) Row/Column Field IDs
+			case 0x00B5: return "SXLI";        // (pivot table) Line Item Array
+			case 0x00C5: return "SXDI";        // (pivot table) Data Item
+			
 			case 0x00D3: return "OBPROJ";
 			case 0x00DC: return "PARAMQRY";
 			case 0x00DE: return "OLESIZE";
 			case BITMAP_00E9: return "BITMAP";
 			case PHONETICPR_00EF: return "PHONETICPR";
+			case 0x00F1: return "SXEX";        // PivotTable View Extended Information
+			case 0x0100: return "SXVDEX";      // Extended PivotTable View Fields
 
 			case LABELRANGES_015F: return "LABELRANGES";
 			case 0x01BA: return "CODENAME";
@@ -145,14 +155,16 @@ public final class UnknownRecord extends StandardRecord {
 
 			case 0x01C0: return "EXCEL9FILE";
 
-			case 0x0802: return "QSISXTAG";
+			case 0x0802: return "QSISXTAG";   // Pivot Table and Query Table Extensions
 			case 0x0803: return "DBQUERYEXT";
 			case 0x0805: return "TXTQUERY";
+			case 0x0810: return "SXVIEWEX9";  // Pivot Table Extensions
 
 			case 0x0812: return "CONTINUEFRT";
 			case QUICKTIP_0800: return "QUICKTIP";
 			case SHEETEXT_0862: return "SHEETEXT";
 			case 0x0863: return "BOOKEXT";
+			case 0x0864: return "SXADDL";    // Pivot Table Additional Info
 			case SHEETPROTECTION_0867: return "SHEETPROTECTION";
 			case RANGEPROTECTION_0868: return "RANGEPROTECTION";
 			case 0x086B: return "DATALABEXTCONTENTS";
diff --git a/src/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java b/src/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java
index 075e3ae5d2..5d249640ef 100644
--- a/src/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java
+++ b/src/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java
@@ -26,6 +26,7 @@ import java.util.TreeMap;
 import org.apache.poi.hssf.model.RecordStream;
 import org.apache.poi.hssf.record.ArrayRecord;
 import org.apache.poi.hssf.record.CellValueRecordInterface;
+import org.apache.poi.hssf.record.ContinueRecord;
 import org.apache.poi.hssf.record.DBCellRecord;
 import org.apache.poi.hssf.record.FormulaRecord;
 import org.apache.poi.hssf.record.IndexRecord;
@@ -43,473 +44,447 @@ import org.apache.poi.hssf.record.formula.FormulaShifter;
  * @author Jason Height (jheight at chariot dot net dot au)
  */
 public final class RowRecordsAggregate extends RecordAggregate {
-    private int _firstrow = -1;
-    private int _lastrow  = -1;
-    private final Map _rowRecords;
-    private final ValueRecordsAggregate _valuesAgg;
-    private final List _unknownRecords;
-    private final SharedValueManager _sharedValueManager;
-
-    /** Creates a new instance of ValueRecordsAggregate */
-    public RowRecordsAggregate() {
-        this(SharedValueManager.EMPTY);
-    }
-    private RowRecordsAggregate(SharedValueManager svm) {
-        _rowRecords = new TreeMap();
-        _valuesAgg = new ValueRecordsAggregate();
-        _unknownRecords = new ArrayList();
-        _sharedValueManager = svm;
-    }
-
-    /**
-     * @param rs record stream with all {@link SharedFormulaRecord}
-     * {@link ArrayRecord}, {@link TableRecord} {@link MergeCellsRecord} Records removed
-     */
-    public RowRecordsAggregate(RecordStream rs, SharedValueManager svm) {
-        this(svm);
-        while(rs.hasNext()) {
-            Record rec = rs.getNext();
-            switch (rec.getSid()) {
-                case RowRecord.sid:
-                    insertRow((RowRecord) rec);
-                    continue;
-                case DBCellRecord.sid:
-                    // end of 'Row Block'.  Should only occur after cell records
-                    // ignore DBCELL records because POI generates them upon re-serialization
-                    continue;
-            }
-            if (rec instanceof UnknownRecord) {
-                addUnknownRecord((UnknownRecord)rec);
-                // might need to keep track of where exactly these belong
-                continue;
-            }
-            if (!(rec instanceof CellValueRecordInterface)) {
-                throw new RuntimeException("Unexpected record type (" + rec.getClass().getName() + ")");
-            }
-            _valuesAgg.construct((CellValueRecordInterface)rec, rs, svm);
-        }
-    }
-    /**
-     * Handles UnknownRecords which appear within the row/cell records
-     */
-    private void addUnknownRecord(UnknownRecord rec) {
-        // ony a few distinct record IDs are encountered by the existing POI test cases:
-        // 0x1065 // many
-        // 0x01C2 // several
-        // 0x0034 // few
-        // No documentation could be found for these
-
-        // keep the unknown records for re-serialization
-        _unknownRecords.add(rec);
-    }
-    public void insertRow(RowRecord row) {
-        // Integer integer = new Integer(row.getRowNumber());
-        _rowRecords.put(new Integer(row.getRowNumber()), row);
-        if ((row.getRowNumber() < _firstrow) || (_firstrow == -1))
-        {
-            _firstrow = row.getRowNumber();
-        }
-        if ((row.getRowNumber() > _lastrow) || (_lastrow == -1))
-        {
-            _lastrow = row.getRowNumber();
-        }
-    }
-
-    public void removeRow(RowRecord row) {
-        int rowIndex = row.getRowNumber();
-        _valuesAgg.removeAllCellsValuesForRow(rowIndex);
-        Integer key = new Integer(rowIndex);
-        RowRecord rr = (RowRecord) _rowRecords.remove(key);
-        if (rr == null) {
-            throw new RuntimeException("Invalid row index (" + key.intValue() + ")");
-        }
-        if (row != rr) {
-            _rowRecords.put(key, rr);
-            throw new RuntimeException("Attempt to remove row that does not belong to this sheet");
-        }
-    }
-
-    public RowRecord getRow(int rowIndex) {
-        if (rowIndex < 0 || rowIndex > 65535) {
-            throw new IllegalArgumentException("The row number must be between 0 and 65535");
-        }
-        return (RowRecord) _rowRecords.get(new Integer(rowIndex));
-    }
-
-    public int getPhysicalNumberOfRows()
-    {
-        return _rowRecords.size();
-    }
-
-    public int getFirstRowNum()
-    {
-        return _firstrow;
-    }
-
-    public int getLastRowNum()
-    {
-        return _lastrow;
-    }
-
-    /** Returns the number of row blocks.
-     * <p/>The row blocks are goupings of rows that contain the DBCell record
-     * after them
-     */
-    public int getRowBlockCount() {
-      int size = _rowRecords.size()/DBCellRecord.BLOCK_SIZE;
-      if ((_rowRecords.size() % DBCellRecord.BLOCK_SIZE) != 0)
-          size++;
-      return size;
-    }
-
-    private int getRowBlockSize(int block) {
-      return RowRecord.ENCODED_SIZE * getRowCountForBlock(block);
-    }
-
-    /** Returns the number of physical rows within a block*/
-    public int getRowCountForBlock(int block) {
-      int startIndex = block * DBCellRecord.BLOCK_SIZE;
-      int endIndex = startIndex + DBCellRecord.BLOCK_SIZE - 1;
-      if (endIndex >= _rowRecords.size())
-        endIndex = _rowRecords.size()-1;
-
-      return endIndex-startIndex+1;
-    }
-
-    /** Returns the physical row number of the first row in a block*/
-    private int getStartRowNumberForBlock(int block) {
-      //Given that we basically iterate through the rows in order,
-      // TODO - For a performance improvement, it would be better to return an instance of
-      //an iterator and use that instance throughout, rather than recreating one and
-      //having to move it to the right position.
-      int startIndex = block * DBCellRecord.BLOCK_SIZE;
-      Iterator rowIter = _rowRecords.values().iterator();
-      RowRecord row = null;
-      //Position the iterator at the start of the block
-      for (int i=0; i<=startIndex;i++) {
-        row = (RowRecord)rowIter.next();
-      }
-      if (row == null) {
-          throw new RuntimeException("Did not find start row for block " + block);
-      }
-
-      return row.getRowNumber();
-    }
-
-    /** Returns the physical row number of the end row in a block*/
-    private int getEndRowNumberForBlock(int block) {
-      int endIndex = ((block + 1)*DBCellRecord.BLOCK_SIZE)-1;
-      if (endIndex >= _rowRecords.size())
-        endIndex = _rowRecords.size()-1;
-
-      Iterator rowIter = _rowRecords.values().iterator();
-      RowRecord row = null;
-      for (int i=0; i<=endIndex;i++) {
-        row = (RowRecord)rowIter.next();
-      }
-      if (row == null) {
-          throw new RuntimeException("Did not find start row for block " + block);
-      }
-      return row.getRowNumber();
-    }
-
-    private int visitRowRecordsForBlock(int blockIndex, RecordVisitor rv) {
-        final int startIndex = blockIndex*DBCellRecord.BLOCK_SIZE;
-        final int endIndex = startIndex + DBCellRecord.BLOCK_SIZE;
-
-        Iterator rowIterator = _rowRecords.values().iterator();
-
-        //Given that we basically iterate through the rows in order,
-        //For a performance improvement, it would be better to return an instance of
-        //an iterator and use that instance throughout, rather than recreating one and
-        //having to move it to the right position.
-        int i=0;
-        for (;i<startIndex;i++)
-          rowIterator.next();
-        int result = 0;
-        while(rowIterator.hasNext() && (i++ < endIndex)) {
-          Record rec = (Record)rowIterator.next();
-          result += rec.getRecordSize();
-          rv.visitRecord(rec);
-        }
-        return result;
-    }
-
-    public void visitContainedRecords(RecordVisitor rv) {
-
-        PositionTrackingVisitor stv = new PositionTrackingVisitor(rv, 0);
-        //DBCells are serialized before row records.
-        final int blockCount = getRowBlockCount();
-        for (int blockIndex = 0; blockIndex < blockCount; blockIndex++) {
-            // Serialize a block of rows.
-            // Hold onto the position of the first row in the block
-            int pos=0;
-            // Hold onto the size of this block that was serialized
-            final int rowBlockSize = visitRowRecordsForBlock(blockIndex, rv);
-            pos += rowBlockSize;
-            // Serialize a block of cells for those rows
-            final int startRowNumber = getStartRowNumberForBlock(blockIndex);
-            final int endRowNumber = getEndRowNumberForBlock(blockIndex);
-            DBCellRecord.Builder dbcrBuilder = new DBCellRecord.Builder();
-            // Note: Cell references start from the second row...
-            int cellRefOffset = (rowBlockSize - RowRecord.ENCODED_SIZE);
-            for (int row = startRowNumber; row <= endRowNumber; row++) {
-                if (_valuesAgg.rowHasCells(row)) {
-                    stv.setPosition(0);
-                    _valuesAgg.visitCellsForRow(row, stv);
-                    int rowCellSize = stv.getPosition();
-                    pos += rowCellSize;
-                    // Add the offset to the first cell for the row into the
-                    // DBCellRecord.
-                    dbcrBuilder.addCellOffset(cellRefOffset);
-                    cellRefOffset = rowCellSize;
-                }
-            }
-            // Calculate Offset from the start of a DBCellRecord to the first Row
-            rv.visitRecord(dbcrBuilder.build(pos));
-        }
-        for (int i=0; i< _unknownRecords.size(); i++) {
-            // Potentially breaking the file here since we don't know exactly where to write these records
-            rv.visitRecord((Record) _unknownRecords.get(i));
-        }
-    }
-
-    public Iterator getIterator() {
-        return _rowRecords.values().iterator();
-    }
-
-    public Iterator getAllRecordsIterator() {
-        List result = new ArrayList(_rowRecords.size() * 2);
-        result.addAll(_rowRecords.values());
-        Iterator vi = _valuesAgg.getIterator();
-        while (vi.hasNext()) {
-            result.add(vi.next());
-        }
-        return result.iterator();
-    }
-
-    public int findStartOfRowOutlineGroup(int row)
-    {
-        // Find the start of the group.
-        RowRecord rowRecord = this.getRow( row );
-        int level = rowRecord.getOutlineLevel();
-        int currentRow = row;
-        while (this.getRow( currentRow ) != null)
-        {
-            rowRecord = this.getRow( currentRow );
-            if (rowRecord.getOutlineLevel() < level)
-                return currentRow + 1;
-            currentRow--;
-        }
-
-        return currentRow + 1;
-    }
-
-    public int findEndOfRowOutlineGroup( int row )
-    {
-        int level = getRow( row ).getOutlineLevel();
-        int currentRow;
-        for (currentRow = row; currentRow < this.getLastRowNum(); currentRow++)
-        {
-            if (getRow(currentRow) == null || getRow(currentRow).getOutlineLevel() < level)
-            {
-                break;
-            }
-        }
-
-        return currentRow-1;
-    }
-
-    /**
-     * Hide all rows at or below the current outline level
-     * @return index of the <em>next<em> row after the last row that gets hidden
-     */
-    private int writeHidden(RowRecord pRowRecord, int row) {
-        int rowIx = row;
-        RowRecord rowRecord = pRowRecord;
-        int level = rowRecord.getOutlineLevel();
-        while (rowRecord != null && getRow(rowIx).getOutlineLevel() >= level) {
-            rowRecord.setZeroHeight(true);
-            rowIx++;
-            rowRecord = getRow(rowIx);
-        }
-        return rowIx;
-    }
-
-    public void collapseRow(int rowNumber) {
-
-        // Find the start of the group.
-        int startRow = findStartOfRowOutlineGroup(rowNumber);
-        RowRecord rowRecord = getRow(startRow);
-
-        // Hide all the columns until the end of the group
-        int nextRowIx = writeHidden(rowRecord, startRow);
-
-        RowRecord row = getRow(nextRowIx);
-        if (row == null) {
-            row = createRow(nextRowIx);
-            insertRow(row);
-        }
-        // Write collapse field
-        row.setColapsed(true);
-    }
-
-    /**
-     * Create a row record.
-     *
-     * @param rowNumber row number
-     * @return RowRecord created for the passed in row number
-     * @see org.apache.poi.hssf.record.RowRecord
-     */
-    public static RowRecord createRow(int rowNumber) {
-        return new RowRecord(rowNumber);
-    }
-
-    public boolean isRowGroupCollapsed( int row )
-    {
-        int collapseRow = findEndOfRowOutlineGroup( row ) + 1;
-
-        if (getRow(collapseRow) == null)
-            return false;
-        else
-            return getRow( collapseRow ).getColapsed();
-    }
-
-    public void expandRow( int rowNumber )
-    {
-        int idx = rowNumber;
-        if (idx == -1)
-            return;
-
-        // If it is already expanded do nothing.
-        if (!isRowGroupCollapsed(idx))
-            return;
-
-        // Find the start of the group.
-        int startIdx = findStartOfRowOutlineGroup( idx );
-        RowRecord row = getRow( startIdx );
-
-        // Find the end of the group.
-        int endIdx = findEndOfRowOutlineGroup( idx );
-
-        // expand:
-        // collapsed bit must be unset
-        // hidden bit gets unset _if_ surrounding groups are expanded you can determine
-        //   this by looking at the hidden bit of the enclosing group.  You will have
-        //   to look at the start and the end of the current group to determine which
-        //   is the enclosing group
-        // hidden bit only is altered for this outline level.  ie.  don't un-collapse contained groups
-        if ( !isRowGroupHiddenByParent( idx ) )
-        {
-            for ( int i = startIdx; i <= endIdx; i++ )
-            {
-                if ( row.getOutlineLevel() == getRow( i ).getOutlineLevel() )
-                    getRow( i ).setZeroHeight( false );
-                else if (!isRowGroupCollapsed(i))
-                    getRow( i ).setZeroHeight( false );
-            }
-        }
-
-        // Write collapse field
-        getRow( endIdx + 1 ).setColapsed( false );
-    }
-
-    public boolean isRowGroupHiddenByParent( int row )
-    {
-        // Look out outline details of end
-        int endLevel;
-        boolean endHidden;
-        int endOfOutlineGroupIdx = findEndOfRowOutlineGroup( row );
-        if (getRow( endOfOutlineGroupIdx + 1 ) == null)
-        {
-            endLevel = 0;
-            endHidden = false;
-        }
-        else
-        {
-            endLevel = getRow( endOfOutlineGroupIdx + 1).getOutlineLevel();
-            endHidden = getRow( endOfOutlineGroupIdx + 1).getZeroHeight();
-        }
-
-        // Look out outline details of start
-        int startLevel;
-        boolean startHidden;
-        int startOfOutlineGroupIdx = findStartOfRowOutlineGroup( row );
-        if (startOfOutlineGroupIdx - 1 < 0 || getRow(startOfOutlineGroupIdx - 1) == null)
-        {
-            startLevel = 0;
-            startHidden = false;
-        }
-        else
-        {
-            startLevel = getRow( startOfOutlineGroupIdx - 1).getOutlineLevel();
-            startHidden = getRow( startOfOutlineGroupIdx - 1 ).getZeroHeight();
-        }
-
-        if (endLevel > startLevel)
-        {
-            return endHidden;
-        }
-        else
-        {
-            return startHidden;
-        }
-    }
-
-    public CellValueRecordInterface[] getValueRecords() {
-        return _valuesAgg.getValueRecords();
-    }
-
-    public IndexRecord createIndexRecord(int indexRecordOffset, int sizeOfInitialSheetRecords) {
-        IndexRecord result = new IndexRecord();
-        result.setFirstRow(_firstrow);
-        result.setLastRowAdd1(_lastrow + 1);
-        // Calculate the size of the records from the end of the BOF
-        // and up to the RowRecordsAggregate...
-
-        // 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 = 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 += getRowBlockSize(block);
-            // account for cell value records after those
-            currentOffset += _valuesAgg.getRowCellBlockSize(
-                    getStartRowNumberForBlock(block), getEndRowNumberForBlock(block));
-
-            // currentOffset is now the location of the DBCELL record for this row-block
-            result.addDbcell(currentOffset);
-            // Add space required to write the DBCELL record (whose reference was just added).
-            currentOffset += (8 + (getRowCountForBlock(block) * 2));
-        }
-        return result;
-    }
-    public void insertCell(CellValueRecordInterface cvRec) {
-        _valuesAgg.insertCell(cvRec);
-    }
-    public void removeCell(CellValueRecordInterface cvRec) {
-        if (cvRec instanceof FormulaRecordAggregate) {
-            ((FormulaRecordAggregate)cvRec).notifyFormulaChanging();
-        }
-        _valuesAgg.removeCell(cvRec);
-    }
-    public FormulaRecordAggregate createFormula(int row, int col) {
-        FormulaRecord fr = new FormulaRecord();
-        fr.setRow(row);
-        fr.setColumn((short) col);
-        return new FormulaRecordAggregate(fr, null, _sharedValueManager);
-    }
-    public void updateFormulasAfterRowShift(FormulaShifter formulaShifter, int currentExternSheetIndex) {
-        _valuesAgg.updateFormulasAfterRowShift(formulaShifter, currentExternSheetIndex);
-    }
+	private int _firstrow = -1;
+	private int _lastrow  = -1;
+	private final Map<Integer, RowRecord> _rowRecords;
+	private final ValueRecordsAggregate _valuesAgg;
+	private final List<Record> _unknownRecords;
+	private final SharedValueManager _sharedValueManager;
+
+	/** Creates a new instance of ValueRecordsAggregate */
+	public RowRecordsAggregate() {
+		this(SharedValueManager.EMPTY);
+	}
+	private RowRecordsAggregate(SharedValueManager svm) {
+		_rowRecords = new TreeMap<Integer, RowRecord>();
+		_valuesAgg = new ValueRecordsAggregate();
+		_unknownRecords = new ArrayList<Record>();
+		_sharedValueManager = svm;
+	}
+
+	/**
+	 * @param rs record stream with all {@link SharedFormulaRecord}
+	 * {@link ArrayRecord}, {@link TableRecord} {@link MergeCellsRecord} Records removed
+	 */
+	public RowRecordsAggregate(RecordStream rs, SharedValueManager svm) {
+		this(svm);
+		while(rs.hasNext()) {
+			Record rec = rs.getNext();
+			switch (rec.getSid()) {
+				case RowRecord.sid:
+					insertRow((RowRecord) rec);
+					continue;
+				case DBCellRecord.sid:
+					// end of 'Row Block'.  Should only occur after cell records
+					// ignore DBCELL records because POI generates them upon re-serialization
+					continue;
+			}
+			if (rec instanceof UnknownRecord) {
+				// might need to keep track of where exactly these belong
+				addUnknownRecord(rec);
+				while (rs.peekNextSid() == ContinueRecord.sid) {
+					addUnknownRecord(rs.getNext());
+				}
+				continue;
+			}
+			if (!(rec instanceof CellValueRecordInterface)) {
+				throw new RuntimeException("Unexpected record type (" + rec.getClass().getName() + ")");
+			}
+			_valuesAgg.construct((CellValueRecordInterface)rec, rs, svm);
+		}
+	}
+	/**
+	 * Handles UnknownRecords which appear within the row/cell records
+	 */
+	private void addUnknownRecord(Record rec) {
+		// ony a few distinct record IDs are encountered by the existing POI test cases:
+		// 0x1065 // many
+		// 0x01C2 // several
+		// 0x0034 // few
+		// No documentation could be found for these
+
+		// keep the unknown records for re-serialization
+		_unknownRecords.add(rec);
+	}
+	public void insertRow(RowRecord row) {
+		// Integer integer = new Integer(row.getRowNumber());
+		_rowRecords.put(new Integer(row.getRowNumber()), row);
+		if ((row.getRowNumber() < _firstrow) || (_firstrow == -1)) {
+			_firstrow = row.getRowNumber();
+		}
+		if ((row.getRowNumber() > _lastrow) || (_lastrow == -1)) {
+			_lastrow = row.getRowNumber();
+		}
+	}
+
+	public void removeRow(RowRecord row) {
+		int rowIndex = row.getRowNumber();
+		_valuesAgg.removeAllCellsValuesForRow(rowIndex);
+		Integer key = new Integer(rowIndex);
+		RowRecord rr = _rowRecords.remove(key);
+		if (rr == null) {
+			throw new RuntimeException("Invalid row index (" + key.intValue() + ")");
+		}
+		if (row != rr) {
+			_rowRecords.put(key, rr);
+			throw new RuntimeException("Attempt to remove row that does not belong to this sheet");
+		}
+	}
+
+	public RowRecord getRow(int rowIndex) {
+		if (rowIndex < 0 || rowIndex > 65535) {
+			throw new IllegalArgumentException("The row number must be between 0 and 65535");
+		}
+		return _rowRecords.get(new Integer(rowIndex));
+	}
+
+	public int getPhysicalNumberOfRows()
+	{
+		return _rowRecords.size();
+	}
+
+	public int getFirstRowNum()
+	{
+		return _firstrow;
+	}
+
+	public int getLastRowNum()
+	{
+		return _lastrow;
+	}
+
+	/** Returns the number of row blocks.
+	 * <p/>The row blocks are goupings of rows that contain the DBCell record
+	 * after them
+	 */
+	public int getRowBlockCount() {
+	  int size = _rowRecords.size()/DBCellRecord.BLOCK_SIZE;
+	  if ((_rowRecords.size() % DBCellRecord.BLOCK_SIZE) != 0)
+		  size++;
+	  return size;
+	}
+
+	private int getRowBlockSize(int block) {
+	  return RowRecord.ENCODED_SIZE * getRowCountForBlock(block);
+	}
+
+	/** Returns the number of physical rows within a block*/
+	public int getRowCountForBlock(int block) {
+	  int startIndex = block * DBCellRecord.BLOCK_SIZE;
+	  int endIndex = startIndex + DBCellRecord.BLOCK_SIZE - 1;
+	  if (endIndex >= _rowRecords.size())
+		endIndex = _rowRecords.size()-1;
+
+	  return endIndex-startIndex+1;
+	}
+
+	/** Returns the physical row number of the first row in a block*/
+	private int getStartRowNumberForBlock(int block) {
+	  //Given that we basically iterate through the rows in order,
+	  // TODO - For a performance improvement, it would be better to return an instance of
+	  //an iterator and use that instance throughout, rather than recreating one and
+	  //having to move it to the right position.
+	  int startIndex = block * DBCellRecord.BLOCK_SIZE;
+	  Iterator rowIter = _rowRecords.values().iterator();
+	  RowRecord row = null;
+	  //Position the iterator at the start of the block
+	  for (int i=0; i<=startIndex;i++) {
+		row = (RowRecord)rowIter.next();
+	  }
+	  if (row == null) {
+		  throw new RuntimeException("Did not find start row for block " + block);
+	  }
+
+	  return row.getRowNumber();
+	}
+
+	/** Returns the physical row number of the end row in a block*/
+	private int getEndRowNumberForBlock(int block) {
+	  int endIndex = ((block + 1)*DBCellRecord.BLOCK_SIZE)-1;
+	  if (endIndex >= _rowRecords.size())
+		endIndex = _rowRecords.size()-1;
+
+	  Iterator rowIter = _rowRecords.values().iterator();
+	  RowRecord row = null;
+	  for (int i=0; i<=endIndex;i++) {
+		row = (RowRecord)rowIter.next();
+	  }
+	  if (row == null) {
+		  throw new RuntimeException("Did not find start row for block " + block);
+	  }
+	  return row.getRowNumber();
+	}
+
+	private int visitRowRecordsForBlock(int blockIndex, RecordVisitor rv) {
+		final int startIndex = blockIndex*DBCellRecord.BLOCK_SIZE;
+		final int endIndex = startIndex + DBCellRecord.BLOCK_SIZE;
+
+		Iterator rowIterator = _rowRecords.values().iterator();
+
+		//Given that we basically iterate through the rows in order,
+		//For a performance improvement, it would be better to return an instance of
+		//an iterator and use that instance throughout, rather than recreating one and
+		//having to move it to the right position.
+		int i=0;
+		for (;i<startIndex;i++)
+		  rowIterator.next();
+		int result = 0;
+		while(rowIterator.hasNext() && (i++ < endIndex)) {
+		  Record rec = (Record)rowIterator.next();
+		  result += rec.getRecordSize();
+		  rv.visitRecord(rec);
+		}
+		return result;
+	}
+
+	public void visitContainedRecords(RecordVisitor rv) {
+
+		PositionTrackingVisitor stv = new PositionTrackingVisitor(rv, 0);
+		//DBCells are serialized before row records.
+		final int blockCount = getRowBlockCount();
+		for (int blockIndex = 0; blockIndex < blockCount; blockIndex++) {
+			// Serialize a block of rows.
+			// Hold onto the position of the first row in the block
+			int pos=0;
+			// Hold onto the size of this block that was serialized
+			final int rowBlockSize = visitRowRecordsForBlock(blockIndex, rv);
+			pos += rowBlockSize;
+			// Serialize a block of cells for those rows
+			final int startRowNumber = getStartRowNumberForBlock(blockIndex);
+			final int endRowNumber = getEndRowNumberForBlock(blockIndex);
+			DBCellRecord.Builder dbcrBuilder = new DBCellRecord.Builder();
+			// Note: Cell references start from the second row...
+			int cellRefOffset = (rowBlockSize - RowRecord.ENCODED_SIZE);
+			for (int row = startRowNumber; row <= endRowNumber; row++) {
+				if (_valuesAgg.rowHasCells(row)) {
+					stv.setPosition(0);
+					_valuesAgg.visitCellsForRow(row, stv);
+					int rowCellSize = stv.getPosition();
+					pos += rowCellSize;
+					// Add the offset to the first cell for the row into the
+					// DBCellRecord.
+					dbcrBuilder.addCellOffset(cellRefOffset);
+					cellRefOffset = rowCellSize;
+				}
+			}
+			// Calculate Offset from the start of a DBCellRecord to the first Row
+			rv.visitRecord(dbcrBuilder.build(pos));
+		}
+		for (int i=0; i< _unknownRecords.size(); i++) {
+			// Potentially breaking the file here since we don't know exactly where to write these records
+			rv.visitRecord(_unknownRecords.get(i));
+		}
+	}
+
+	public Iterator getIterator() {
+		return _rowRecords.values().iterator();
+	}
+
+	public int findStartOfRowOutlineGroup(int row) {
+		// Find the start of the group.
+		RowRecord rowRecord = this.getRow( row );
+		int level = rowRecord.getOutlineLevel();
+		int currentRow = row;
+		while (this.getRow( currentRow ) != null) {
+			rowRecord = this.getRow( currentRow );
+			if (rowRecord.getOutlineLevel() < level) {
+				return currentRow + 1;
+			}
+			currentRow--;
+		}
+
+		return currentRow + 1;
+	}
+
+	public int findEndOfRowOutlineGroup(int row) {
+		int level = getRow( row ).getOutlineLevel();
+		int currentRow;
+		for (currentRow = row; currentRow < getLastRowNum(); currentRow++) {
+			if (getRow(currentRow) == null || getRow(currentRow).getOutlineLevel() < level) {
+				break;
+			}
+		}
+
+		return currentRow-1;
+	}
+
+	/**
+	 * Hide all rows at or below the current outline level
+	 * @return index of the <em>next<em> row after the last row that gets hidden
+	 */
+	private int writeHidden(RowRecord pRowRecord, int row) {
+		int rowIx = row;
+		RowRecord rowRecord = pRowRecord;
+		int level = rowRecord.getOutlineLevel();
+		while (rowRecord != null && getRow(rowIx).getOutlineLevel() >= level) {
+			rowRecord.setZeroHeight(true);
+			rowIx++;
+			rowRecord = getRow(rowIx);
+		}
+		return rowIx;
+	}
+
+	public void collapseRow(int rowNumber) {
+
+		// Find the start of the group.
+		int startRow = findStartOfRowOutlineGroup(rowNumber);
+		RowRecord rowRecord = getRow(startRow);
+
+		// Hide all the columns until the end of the group
+		int nextRowIx = writeHidden(rowRecord, startRow);
+
+		RowRecord row = getRow(nextRowIx);
+		if (row == null) {
+			row = createRow(nextRowIx);
+			insertRow(row);
+		}
+		// Write collapse field
+		row.setColapsed(true);
+	}
+
+	/**
+	 * Create a row record.
+	 *
+	 * @param rowNumber row number
+	 * @return RowRecord created for the passed in row number
+	 * @see org.apache.poi.hssf.record.RowRecord
+	 */
+	public static RowRecord createRow(int rowNumber) {
+		return new RowRecord(rowNumber);
+	}
+
+	public boolean isRowGroupCollapsed(int row) {
+		int collapseRow = findEndOfRowOutlineGroup(row) + 1;
+
+		if (getRow(collapseRow) == null) {
+			return false;
+		}
+		return getRow( collapseRow ).getColapsed();
+	}
+
+	public void expandRow(int rowNumber) {
+		int idx = rowNumber;
+		if (idx == -1)
+			return;
+
+		// If it is already expanded do nothing.
+		if (!isRowGroupCollapsed(idx)) {
+			return;
+		}
+
+		// Find the start of the group.
+		int startIdx = findStartOfRowOutlineGroup(idx);
+		RowRecord row = getRow(startIdx);
+
+		// Find the end of the group.
+		int endIdx = findEndOfRowOutlineGroup(idx);
+
+		// expand:
+		// collapsed bit must be unset
+		// hidden bit gets unset _if_ surrounding groups are expanded you can determine
+		//   this by looking at the hidden bit of the enclosing group.  You will have
+		//   to look at the start and the end of the current group to determine which
+		//   is the enclosing group
+		// hidden bit only is altered for this outline level.  ie.  don't un-collapse contained groups
+		if (!isRowGroupHiddenByParent(idx)) {
+			for (int i = startIdx; i <= endIdx; i++) {
+				RowRecord otherRow = getRow(i);
+				if (row.getOutlineLevel() == otherRow.getOutlineLevel() || !isRowGroupCollapsed(i)) {
+					otherRow.setZeroHeight(false);
+				}
+			}
+		}
+
+		// Write collapse field
+		getRow(endIdx + 1).setColapsed(false);
+	}
+
+	public boolean isRowGroupHiddenByParent(int row) {
+		// Look out outline details of end
+		int endLevel;
+		boolean endHidden;
+		int endOfOutlineGroupIdx = findEndOfRowOutlineGroup(row);
+		if (getRow(endOfOutlineGroupIdx + 1) == null) {
+			endLevel = 0;
+			endHidden = false;
+		} else {
+			endLevel = getRow(endOfOutlineGroupIdx + 1).getOutlineLevel();
+			endHidden = getRow(endOfOutlineGroupIdx + 1).getZeroHeight();
+		}
+
+		// Look out outline details of start
+		int startLevel;
+		boolean startHidden;
+		int startOfOutlineGroupIdx = findStartOfRowOutlineGroup( row );
+		if (startOfOutlineGroupIdx - 1 < 0 || getRow(startOfOutlineGroupIdx - 1) == null) {
+			startLevel = 0;
+			startHidden = false;
+		} else {
+			startLevel = getRow(startOfOutlineGroupIdx - 1).getOutlineLevel();
+			startHidden = getRow(startOfOutlineGroupIdx - 1).getZeroHeight();
+		}
+
+		if (endLevel > startLevel) {
+			return endHidden;
+		}
+
+		return startHidden;
+	}
+
+	public CellValueRecordInterface[] getValueRecords() {
+		return _valuesAgg.getValueRecords();
+	}
+
+	public IndexRecord createIndexRecord(int indexRecordOffset, int sizeOfInitialSheetRecords) {
+		IndexRecord result = new IndexRecord();
+		result.setFirstRow(_firstrow);
+		result.setLastRowAdd1(_lastrow + 1);
+		// Calculate the size of the records from the end of the BOF
+		// and up to the RowRecordsAggregate...
+
+		// 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 = 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 += getRowBlockSize(block);
+			// account for cell value records after those
+			currentOffset += _valuesAgg.getRowCellBlockSize(
+					getStartRowNumberForBlock(block), getEndRowNumberForBlock(block));
+
+			// currentOffset is now the location of the DBCELL record for this row-block
+			result.addDbcell(currentOffset);
+			// Add space required to write the DBCELL record (whose reference was just added).
+			currentOffset += (8 + (getRowCountForBlock(block) * 2));
+		}
+		return result;
+	}
+	public void insertCell(CellValueRecordInterface cvRec) {
+		_valuesAgg.insertCell(cvRec);
+	}
+	public void removeCell(CellValueRecordInterface cvRec) {
+		if (cvRec instanceof FormulaRecordAggregate) {
+			((FormulaRecordAggregate)cvRec).notifyFormulaChanging();
+		}
+		_valuesAgg.removeCell(cvRec);
+	}
+	public FormulaRecordAggregate createFormula(int row, int col) {
+		FormulaRecord fr = new FormulaRecord();
+		fr.setRow(row);
+		fr.setColumn((short) col);
+		return new FormulaRecordAggregate(fr, null, _sharedValueManager);
+	}
+	public void updateFormulasAfterRowShift(FormulaShifter formulaShifter, int currentExternSheetIndex) {
+		_valuesAgg.updateFormulasAfterRowShift(formulaShifter, currentExternSheetIndex);
+	}
 }
diff --git a/src/testcases/org/apache/poi/hssf/model/AllModelTests.java b/src/testcases/org/apache/poi/hssf/model/AllModelTests.java
index f157d3a0f2..35565a7a65 100755
--- a/src/testcases/org/apache/poi/hssf/model/AllModelTests.java
+++ b/src/testcases/org/apache/poi/hssf/model/AllModelTests.java
@@ -35,6 +35,7 @@ public final class AllModelTests {
 		result.addTestSuite(TestFormulaParserEval.class);
 		result.addTestSuite(TestFormulaParserIf.class);
 		result.addTestSuite(TestOperandClassTransformer.class);
+		result.addTestSuite(TestRowBlocksReader.class);
 		result.addTestSuite(TestRVA.class);
 		result.addTestSuite(TestSheet.class);
 		result.addTestSuite(TestSheetAdditional.class);
diff --git a/src/testcases/org/apache/poi/hssf/model/TestRowBlocksReader.java b/src/testcases/org/apache/poi/hssf/model/TestRowBlocksReader.java
new file mode 100644
index 0000000000..652bea1a16
--- /dev/null
+++ b/src/testcases/org/apache/poi/hssf/model/TestRowBlocksReader.java
@@ -0,0 +1,60 @@
+/* ====================================================================

+   Licensed to the Apache Software Foundation (ASF) under one or more

+   contributor license agreements.  See the NOTICE file distributed with

+   this work for additional information regarding copyright ownership.

+   The ASF licenses this file to You under the Apache License, Version 2.0

+   (the "License"); you may not use this file except in compliance with

+   the License.  You may obtain a copy of the License at

+

+       http://www.apache.org/licenses/LICENSE-2.0

+

+   Unless required by applicable law or agreed to in writing, software

+   distributed under the License is distributed on an "AS IS" BASIS,

+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

+   See the License for the specific language governing permissions and

+   limitations under the License.

+==================================================================== */

+

+package org.apache.poi.hssf.model;

+

+import java.util.Arrays;

+

+import junit.framework.AssertionFailedError;

+import junit.framework.TestCase;

+

+import org.apache.poi.hssf.record.NumberRecord;

+import org.apache.poi.hssf.record.Record;

+import org.apache.poi.hssf.record.RowRecord;

+import org.apache.poi.hssf.record.UnknownRecord;

+import org.apache.poi.hssf.record.WindowTwoRecord;

+

+/**

+ * Tests for {@link RowBlocksReader}

+ * 

+ * @author Josh Micich

+ */

+public final class TestRowBlocksReader extends TestCase {

+	public void testAbnormalPivotTableRecords_bug46280() {

+		int SXVIEW_SID = 0x00B0;

+		Record[] inRecs = {

+			new RowRecord(0),

+			new NumberRecord(),

+			// normally MSODRAWING(0x00EC) would come here before SXVIEW

+			new UnknownRecord(SXVIEW_SID, "dummydata (SXVIEW: View Definition)".getBytes()),

+			new WindowTwoRecord(),

+		};

+		RecordStream rs = new RecordStream(Arrays.asList(inRecs), 0);

+		RowBlocksReader rbr = new RowBlocksReader(rs);

+		if (rs.peekNextClass() == WindowTwoRecord.class) {

+			// Should have stopped at the SXVIEW record 

+			throw new AssertionFailedError("Identified bug 46280b");

+		}

+		RecordStream rbStream = rbr.getPlainRecordStream();

+		assertEquals(inRecs[0], rbStream.getNext());

+		assertEquals(inRecs[1], rbStream.getNext());

+		assertFalse(rbStream.hasNext());

+		assertTrue(rs.hasNext());

+		assertEquals(inRecs[2], rs.getNext());

+		assertEquals(inRecs[3], rs.getNext());

+	}

+}

diff --git a/src/testcases/org/apache/poi/hssf/record/aggregates/TestRowRecordsAggregate.java b/src/testcases/org/apache/poi/hssf/record/aggregates/TestRowRecordsAggregate.java
index 7921b2607c..6c0ae1d3ae 100644
--- a/src/testcases/org/apache/poi/hssf/record/aggregates/TestRowRecordsAggregate.java
+++ b/src/testcases/org/apache/poi/hssf/record/aggregates/TestRowRecordsAggregate.java
@@ -21,24 +21,30 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
+import java.util.Arrays;
 
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
+import org.apache.poi.hssf.model.RecordStream;
 import org.apache.poi.hssf.record.ArrayRecord;
+import org.apache.poi.hssf.record.ContinueRecord;
 import org.apache.poi.hssf.record.FormulaRecord;
+import org.apache.poi.hssf.record.NumberRecord;
 import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.RowRecord;
 import org.apache.poi.hssf.record.SharedFormulaRecord;
 import org.apache.poi.hssf.record.SharedValueRecordBase;
 import org.apache.poi.hssf.record.TableRecord;
+import org.apache.poi.hssf.record.UnknownRecord;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.apache.poi.hssf.usermodel.RecordInspector;
+import org.apache.poi.hssf.usermodel.RecordInspector.RecordCollector;
 import org.apache.poi.hssf.util.CellRangeAddress8Bit;
 
 /**
- * 
+ * Tests for {@link RowRecordsAggregate}
  */
 public final class TestRowRecordsAggregate extends TestCase {
 
@@ -113,4 +119,37 @@ public final class TestRowRecordsAggregate extends TestCase {
 		assertEquals(range.getFirstRow(), firstFormula.getRow());
 		assertEquals(range.getFirstColumn(), firstFormula.getColumn());
 	}
+
+	/**
+	 * This problem was noted as the overt symptom of bug 46280.  The logic for skipping {@link
+	 * UnknownRecord}s in the constructor {@link RowRecordsAggregate} did not allow for the
+	 * possibility of tailing {@link ContinueRecord}s.<br/>
+	 * The functionality change being tested here is actually not critical to the overall fix
+	 * for bug 46280, since the fix involved making sure the that offending <i>PivotTable</i>
+	 * records do not get into {@link RowRecordsAggregate}.<br/>
+	 * This fix in {@link RowRecordsAggregate} was implemented anyway since any {@link 
+	 * UnknownRecord} has the potential of being 'continued'.
+	 */
+	public void testUnknownContinue_bug46280() {
+		Record[] inRecs = {
+			new RowRecord(0),
+			new NumberRecord(),
+			new UnknownRecord(0x5555, "dummydata".getBytes()),
+			new ContinueRecord("moredummydata".getBytes()),
+		};
+		RecordStream rs = new RecordStream(Arrays.asList(inRecs), 0);
+		RowRecordsAggregate rra;
+		try {
+			rra = new RowRecordsAggregate(rs, SharedValueManager.EMPTY);
+		} catch (RuntimeException e) {
+			if (e.getMessage().startsWith("Unexpected record type")) {
+				throw new AssertionFailedError("Identified bug 46280a");
+			}
+			throw e;
+		}
+		RecordCollector rv = new RecordCollector();
+		rra.visitContainedRecords(rv);
+		Record[] outRecs = rv.getRecords();
+		assertEquals(5, outRecs.length);
+	}
 }