Fix for bugs 26321 and 44958 - preserve position of ArrayRecords and TableRecords among cell value records

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

src/documentation/content/xdocs/changes.xml

@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">26321 and 44958 - preserve position of ArrayRecords and TableRecords among cell value records</action>
            <action dev="POI-DEVELOPERS" type="fix">Impove empty header or footer handling in HWPF HeaderStories</action>
            <action dev="POI-DEVELOPERS" type="fix">Avoid NPE in hssf.usermodel.HeaderFooter when stripping fields out</action>
            <action dev="POI-DEVELOPERS" type="fix">Avoid NPE in EscherBSERecord on older escher records</action>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">26321 and 44958 - preserve position of ArrayRecords and TableRecords among cell value records</action>
            <action dev="POI-DEVELOPERS" type="fix">Impove empty header or footer handling in HWPF HeaderStories</action>
            <action dev="POI-DEVELOPERS" type="fix">Avoid NPE in hssf.usermodel.HeaderFooter when stripping fields out</action>
            <action dev="POI-DEVELOPERS" type="fix">Avoid NPE in EscherBSERecord on older escher records</action>

src/java/org/apache/poi/hssf/model/RowBlocksReader.java

@@ -0,0 +1,116 @@
+/* ====================================================================
+   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.ArrayList;
+import java.util.List;
+
+import org.apache.poi.hssf.record.ArrayRecord;
+import org.apache.poi.hssf.record.MergeCellsRecord;
+import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.SharedFormulaRecord;
+import org.apache.poi.hssf.record.TableRecord;
+import org.apache.poi.hssf.record.aggregates.MergedCellsTable;
+import org.apache.poi.hssf.record.aggregates.SharedValueManager;
+
+/**
+ * Segregates the 'Row Blocks' section of a single sheet into plain row/cell records and 
+ * shared formula records.
+ * 
+ * @author Josh Micich
+ */
+public final class RowBlocksReader {
+
+	private final List _plainRecords;
+	private final SharedValueManager _sfm;
+	private final MergeCellsRecord[] _mergedCellsRecords;
+	private final int _totalNumberOfRecords;
+
+	/**
+	 * Also collects any loose MergeCellRecords and puts them in the supplied
+	 * mergedCellsTable
+	 */
+	public RowBlocksReader(List recs, int startIx) {
+		List plainRecords = new ArrayList();
+		List shFrmRecords = new ArrayList();
+		List arrayRecords = new ArrayList();
+		List tableRecords = new ArrayList();
+		List mergeCellRecords = new ArrayList();
+
+		int endIx = -1;
+		for (int i = startIx; i < recs.size(); i++) {
+			Record rec = (Record) recs.get(i);
+			if (RecordOrderer.isEndOfRowBlock(rec.getSid())) {
+				// End of row/cell records for the current sheet
+				// Note - It is important that this code does not inadvertently any sheet 
+				// records from a subsequent sheet.  For example, if SharedFormulaRecords 
+				// are taken from the wrong sheet, this could cause bug 44449. 
+				endIx = i;
+				break;
+			}
+			List dest;
+			switch (rec.getSid()) {
+				case MergeCellsRecord.sid:    dest = mergeCellRecords; break;
+				case SharedFormulaRecord.sid: dest = shFrmRecords;     break;
+				case ArrayRecord.sid:         dest = arrayRecords;     break;
+				case TableRecord.sid:         dest = tableRecords;     break;
+				default:                      dest = plainRecords;
+			}
+			dest.add(rec);
+		}
+		if (endIx < 0) {
+			throw new RuntimeException("Failed to find end of row/cell records");
+		}
+		SharedFormulaRecord[] sharedFormulaRecs = new SharedFormulaRecord[shFrmRecords.size()];
+		ArrayRecord[] arrayRecs = new ArrayRecord[arrayRecords.size()];
+		TableRecord[] tableRecs = new TableRecord[tableRecords.size()];
+		shFrmRecords.toArray(sharedFormulaRecs);
+		arrayRecords.toArray(arrayRecs);
+		tableRecords.toArray(tableRecs);
+		
+		_plainRecords = plainRecords;
+		_sfm = SharedValueManager.create(sharedFormulaRecs, arrayRecs, tableRecs);
+		_mergedCellsRecords = new MergeCellsRecord[mergeCellRecords.size()];
+		mergeCellRecords.toArray(_mergedCellsRecords);
+		_totalNumberOfRecords = endIx - startIx;
+	}
+	
+	/**
+	 * Some unconventional apps place {@link MergeCellsRecord}s within the row block.  They 
+	 * actually should be in the {@link MergedCellsTable} which is much later (see bug 45699).
+	 * @return any loose  <tt>MergeCellsRecord</tt>s found
+	 */
+	public MergeCellsRecord[] getLooseMergedCells() {
+		return _mergedCellsRecords;
+	}
+
+	public int getTotalNumberOfRecords() {
+		return _totalNumberOfRecords;
+	}
+
+	public SharedValueManager getSharedFormulaManager() {
+		return _sfm;
+	}
+	/**
+	 * @return a {@link RecordStream} containing all the non-{@link SharedFormulaRecord} 
+	 * non-{@link ArrayRecord} and non-{@link TableRecord} Records.
+	 */
+	public RecordStream getPlainRecordStream() {
+		return new RecordStream(_plainRecords, 0);
+	}
+}

src/java/org/apache/poi/hssf/model/Sheet.java

@@ -27,7 +27,6 @@ import org.apache.poi.hssf.record.CalcCountRecord;
 import org.apache.poi.hssf.record.CalcModeRecord;
 import org.apache.poi.hssf.record.CellValueRecordInterface;
 import org.apache.poi.hssf.record.ColumnInfoRecord;
-import org.apache.poi.hssf.record.DBCellRecord;
 import org.apache.poi.hssf.record.DVALRecord;
 import org.apache.poi.hssf.record.DefaultColWidthRecord;
 import org.apache.poi.hssf.record.DefaultRowHeightRecord;
@@ -56,7 +55,6 @@ import org.apache.poi.hssf.record.SCLRecord;
 import org.apache.poi.hssf.record.SaveRecalcRecord;
 import org.apache.poi.hssf.record.ScenarioProtectRecord;
 import org.apache.poi.hssf.record.SelectionRecord;
-import org.apache.poi.hssf.record.StringRecord;
 import org.apache.poi.hssf.record.UncalcedRecord;
 import org.apache.poi.hssf.record.WSBoolRecord;
 import org.apache.poi.hssf.record.WindowTwoRecord;
@@ -64,6 +62,7 @@ import org.apache.poi.hssf.record.aggregates.CFRecordsAggregate;
 import org.apache.poi.hssf.record.aggregates.ColumnInfoRecordsAggregate;
 import org.apache.poi.hssf.record.aggregates.ConditionalFormattingTable;
 import org.apache.poi.hssf.record.aggregates.DataValidityTable;
+import org.apache.poi.hssf.record.aggregates.FormulaRecordAggregate;
 import org.apache.poi.hssf.record.aggregates.MergedCellsTable;
 import org.apache.poi.hssf.record.aggregates.PageSettingsBlock;
 import org.apache.poi.hssf.record.aggregates.RecordAggregate;
@@ -181,17 +180,11 @@ public final class Sheet implements Model {
 
         for (int k = offset; k < inRecs.size(); k++) {
             Record rec = ( Record ) inRecs.get(k);
-            if ( rec.getSid() == DBCellRecord.sid ) {
-                continue;
-            }
             if ( rec.getSid() == IndexRecord.sid ) {
                 // ignore INDEX record because it is only needed by Excel, 
                 // and POI always re-calculates its contents 
                 continue;
             }
-            if ( rec.getSid() == StringRecord.sid ) {
-                continue;
-            }
 
             if ( rec.getSid() == CFHeaderRecord.sid ) {
                 RecordStream rs = new RecordStream(inRecs, k);
@@ -221,10 +214,11 @@ public final class Sheet implements Model {
                 if (retval._rowsAggregate != null) {
                     throw new RuntimeException("row/cell records found in the wrong place");
                 }
-                int lastRowCellRec = findEndOfRowBlock(inRecs, k, retval._mergedCellsTable);
+                RowBlocksReader rbr = new RowBlocksReader(inRecs, k);
-                retval._rowsAggregate = new RowRecordsAggregate(inRecs, k, lastRowCellRec);
+                retval._mergedCellsTable.addRecords(rbr.getLooseMergedCells());
+                retval._rowsAggregate = new RowRecordsAggregate(rbr.getPlainRecordStream(), rbr.getSharedFormulaManager());
                 records.add(retval._rowsAggregate); //only add the aggregate once
-                k = lastRowCellRec -1;
+                k += rbr.getTotalNumberOfRecords() - 1;
                 continue;
             }
              
@@ -344,26 +338,6 @@ public final class Sheet implements Model {
         return retval;
     }
 
-    /**
-     * Also collects any rogue MergeCellRecords
-     * @return the index one after the last row/cell record
-     */
-    private static int findEndOfRowBlock(List recs, int startIx, MergedCellsTable mergedCellsTable) {
-        for(int i=startIx; i<recs.size(); i++) {
-            Record rec = (Record) recs.get(i);
-            if (RecordOrderer.isEndOfRowBlock(rec.getSid())) {
-                return i;
-            }
-            if (rec.getSid() == MergeCellsRecord.sid) {
-                    // Some apps scatter these records between the rows/cells but they are supposed to
-                    // be well after the row/cell records.  We collect them here 
-                    // see bug 45699
-                    mergedCellsTable.add((MergeCellsRecord) rec);
-            }
-        }
-        throw new RuntimeException("Failed to find end of row/cell records");
-    }
-
     private static final class RecordCloner implements RecordVisitor {
 
         private final List _destList;
@@ -618,7 +592,7 @@ public final class Sheet implements Model {
                 //Can there be more than one BOF for a sheet? If not then we can
                 //remove this guard. So be safe it is left here.
                 if (_rowsAggregate != null) {
-                	// find forward distance to first RowRecord
+                    // find forward distance to first RowRecord
                     int initRecsSize = getSizeOfInitialSheetRecords(k);
                     int currentPos = ptv.getPosition();
                     ptv.visitRecord(_rowsAggregate.createIndexRecord(currentPos, initRecsSize));
@@ -1867,4 +1841,8 @@ public final class Sheet implements Model {
         }
         return _dataValidityTable;
     }
+
+    public FormulaRecordAggregate createFormula(int row, int col) {
+        return _rowsAggregate.createFormula(row, col);
+    }
 }

src/java/org/apache/poi/hssf/record/ArrayRecord.java

@@ -18,7 +18,6 @@
 package org.apache.poi.hssf.record;
 
 import org.apache.poi.hssf.record.formula.Ptg;
-import org.apache.poi.hssf.util.CellRangeAddress8Bit;
 import org.apache.poi.util.HexDump;
 import org.apache.poi.util.LittleEndian;
 
@@ -29,20 +28,22 @@ import org.apache.poi.util.LittleEndian;
  * 
  * @author Josh Micich
  */		
-public final class ArrayRecord extends Record {
+public final class ArrayRecord extends SharedValueRecordBase {
 
 	public final static short sid = 0x0221;
 	private static final int OPT_ALWAYS_RECALCULATE = 0x0001;
 	private static final int OPT_CALCULATE_ON_OPEN  = 0x0002;
 	
-	private CellRangeAddress8Bit _range;
-	
 	private int	_options;
 	private int _field3notUsed;
 	private Ptg[] _formulaTokens;
 
 	public ArrayRecord(RecordInputStream in) {
 		super(in);
+		_options = in.readUShort();
+		_field3notUsed = in.readInt();
+		int formulaLen = in.readUShort();
+		_formulaTokens = Ptg.readTokens(formulaLen, in);
 	}
 
 	public boolean isAlwaysRecalculate() {
@@ -52,27 +53,12 @@ public final class ArrayRecord extends Record {
 		return (_options & OPT_CALCULATE_ON_OPEN) != 0;
 	}
 
-	protected void validateSid(short id) {
+	protected int getExtraDataSize() {
-		if (id != sid) {
+		return 2 + 4
-			throw new RecordFormatException("NOT A valid Array RECORD");
+			+ 2 + Ptg.getEncodedSize(_formulaTokens);
-		}
 	}
-
+	protected void serializeExtraData(int offset, byte[] data) {
-	private int getDataSize(){
+		int pos = offset;
-		return CellRangeAddress8Bit.ENCODED_SIZE 
-			+ 2 + 4
-			+ getFormulaSize();
-	}
-
-	public int serialize( int offset, byte[] data ) {
-		int dataSize = getDataSize();
-
-		LittleEndian.putShort(data, 0 + offset, sid);
-		LittleEndian.putUShort(data, 2 + offset, dataSize);
-
-		int pos = offset+4;
-		_range.serialize(pos, data);
-		pos += CellRangeAddress8Bit.ENCODED_SIZE;
 		LittleEndian.putUShort(data, pos, _options);
 		pos+=2;
 		LittleEndian.putInt(data, pos, _field3notUsed);
@@ -81,29 +67,6 @@ public final class ArrayRecord extends Record {
 		LittleEndian.putUShort(data, pos, tokenSize);
 		pos+=2;
 		Ptg.serializePtgs(_formulaTokens, data, pos);
-		return dataSize + 4;
-	}
-
-	private int getFormulaSize() {
-		int result = 0;
-		for (int i = 0; i < _formulaTokens.length; i++) {
-			result += _formulaTokens[i].getSize();
-		}
-		return result;
-	}
-
-
-	public int getRecordSize(){
-		return 4 + getDataSize();
-	}
-
-
-	protected void fillFields(RecordInputStream in) {
-		_range = new CellRangeAddress8Bit(in);
-		_options = in.readUShort();
-		_field3notUsed = in.readInt();
-		int formulaLen = in.readUShort();
-		_formulaTokens = Ptg.readTokens(formulaLen, in);
 	}
 
 	public short getSid() {
@@ -113,12 +76,13 @@ public final class ArrayRecord extends Record {
 	public String toString() {
 		StringBuffer sb = new StringBuffer();
 		sb.append(getClass().getName()).append(" [ARRAY]\n");
-		sb.append(" range=").append(_range.toString()).append("\n");
+		sb.append(" range=").append(getRange().toString()).append("\n");
 		sb.append(" options=").append(HexDump.shortToHex(_options)).append("\n");
 		sb.append(" notUsed=").append(HexDump.intToHex(_field3notUsed)).append("\n");
 		sb.append(" formula:").append("\n");
 		for (int i = 0; i < _formulaTokens.length; i++) {
-			sb.append(_formulaTokens[i].toString());
+			Ptg ptg = _formulaTokens[i];
+			sb.append(ptg.toString()).append(ptg.getRVAType()).append("\n");
 		}
 		sb.append("]");
 		return sb.toString();

src/java/org/apache/poi/hssf/record/FormulaRecord.java

@@ -17,9 +17,6 @@
 
 package org.apache.poi.hssf.record;
 
-import java.util.Arrays;
-import java.util.List;
-
 import org.apache.poi.hssf.record.formula.Ptg;
 import org.apache.poi.util.BitField;
 import org.apache.poi.util.BitFieldFactory;
@@ -27,7 +24,7 @@ import org.apache.poi.util.HexDump;
 import org.apache.poi.util.LittleEndian;
 
 /**
- * Formula Record.
+ * Formula Record (0x0006).
  * REFERENCE:  PG 317/444 Microsoft Excel 97 Developer's Kit (ISBN: 1-57231-498-2)<P>
  * @author Andrew C. Oliver (acoliver at apache dot org)
  * @author Jason Height (jheight at chariot dot net dot au)
@@ -270,7 +267,8 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 
         for (int k = 0; k < field_8_parsed_expr.length; k++ ) {
             sb.append("     Ptg[").append(k).append("]=");
-            sb.append(field_8_parsed_expr[k].toString()).append("\n");
+            Ptg ptg = field_8_parsed_expr[k];
+            sb.append(ptg.toString()).append(ptg.getRVAType()).append("\n");
         }
         sb.append("[/FORMULA]\n");
         return sb.toString();

src/java/org/apache/poi/hssf/record/RecordFactory.java

@@ -41,7 +41,7 @@ import java.util.Set;
  * @author Csaba Nagy (ncsaba at yahoo dot com)
  */
 public final class RecordFactory {
-    private static final int NUM_RECORDS = 512;
+	private static final int NUM_RECORDS = 512;
 
 	private static final Class[] CONSTRUCTOR_ARGS = { RecordInputStream.class, };
 
@@ -50,6 +50,7 @@ public final class RecordFactory {
 	 * Note - this most but not *every* subclass of Record.
 	 */
 	private static final Class[] records = {
+		ArrayRecord.class,
 		BackupRecord.class,
 		BlankRecord.class,
 		BOFRecord.class,

src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java

@@ -22,7 +22,6 @@ import org.apache.poi.hssf.record.formula.AreaPtg;
 import org.apache.poi.hssf.record.formula.Ptg;
 import org.apache.poi.hssf.record.formula.RefNPtg;
 import org.apache.poi.hssf.record.formula.RefPtg;
-import org.apache.poi.hssf.util.CellRangeAddress8Bit;
 import org.apache.poi.util.HexDump;
 
 /**
@@ -36,59 +35,31 @@ import org.apache.poi.util.HexDump;
  * record types.
  * @author Danny Mui at apache dot org
  */
-public final class SharedFormulaRecord extends Record {
+public final class SharedFormulaRecord extends SharedValueRecordBase {
     public final static short   sid = 0x04BC;
 
-    private CellRangeAddress8Bit _range;
     private int field_5_reserved;
     private Ptg[] field_7_parsed_expr;
 
     public SharedFormulaRecord() {
-    	_range = new CellRangeAddress8Bit(0, 0, 0, 0);
+        field_7_parsed_expr = Ptg.EMPTY_PTG_ARRAY;
-    	field_7_parsed_expr = Ptg.EMPTY_PTG_ARRAY;
     }
 
     /**
      * @param in the RecordInputstream to read the record from
      */
     public SharedFormulaRecord(RecordInputStream in) {
-          super(in);
+        super(in);
+        field_5_reserved        = in.readShort();
+        int field_6_expression_len = in.readShort();
+        field_7_parsed_expr = Ptg.readTokens(field_6_expression_len, in);
     }
-
+    protected void serializeExtraData(int offset, byte[] data) {
-    protected void validateSid(short id) {
-        if (id != this.sid) {
-            throw new RecordFormatException("Not a valid SharedFormula");
-        }
-    }
-
-    public int getFirstRow() {
-      return _range.getFirstRow();
-    }
-
-    public int getLastRow() {
-      return _range.getLastRow();
-    }
-
-    public short getFirstColumn() {
-      return (short) _range.getFirstColumn();
-    }
-
-    public short getLastColumn() {
-      return (short) _range.getLastColumn();
-    }
-
-    /**
-     * spit the record out AS IS.  no interpretation or identification
-     */
-
-    public int serialize(int offset, byte [] data)
-    {
         //Because this record is converted to individual Formula records, this method is not required.
         throw new UnsupportedOperationException("Cannot serialize a SharedFormulaRecord");
     }
     
-    public int getRecordSize()
+    protected int getExtraDataSize() {
-    {
         //Because this record is converted to individual Formula records, this method is not required.
         throw new UnsupportedOperationException("Cannot get the size for a SharedFormulaRecord");
 
@@ -103,12 +74,13 @@ public final class SharedFormulaRecord extends Record {
         StringBuffer buffer = new StringBuffer();
 
         buffer.append("[SHARED FORMULA (").append(HexDump.intToHex(sid)).append("]\n");
-        buffer.append("    .range      = ").append(_range.toString()).append("\n");
+        buffer.append("    .range      = ").append(getRange().toString()).append("\n");
         buffer.append("    .reserved    = ").append(HexDump.shortToHex(field_5_reserved)).append("\n");
 
         for (int k = 0; k < field_7_parsed_expr.length; k++ ) {
            buffer.append("Formula[").append(k).append("]");
-           buffer.append(field_7_parsed_expr[k].toString()).append("\n");
+           Ptg ptg = field_7_parsed_expr[k];
+           buffer.append(ptg.toString()).append(ptg.getRVAType()).append("\n");
         }
 
         buffer.append("[/SHARED FORMULA]\n");
@@ -119,23 +91,6 @@ public final class SharedFormulaRecord extends Record {
         return sid;
     }
 
-    protected void fillFields(RecordInputStream in) {
-        _range = new CellRangeAddress8Bit(in);
-        field_5_reserved        = in.readShort();
-        int field_6_expression_len = in.readShort();
-        field_7_parsed_expr = Ptg.readTokens(field_6_expression_len, in);
-    }
-
-    /**
-     * Are we shared by the supplied formula record?
-     */
-    public boolean isFormulaInShared(FormulaRecord formula) {
-      final int formulaRow = formula.getRow();
-      final int formulaColumn = formula.getColumn();
-      return ((getFirstRow() <= formulaRow) && (getLastRow() >= formulaRow) &&
-          (getFirstColumn() <= formulaColumn) && (getLastColumn() >= formulaColumn));
-    }
-
     /**
      * Creates a non shared formula from the shared formula
      * counter part
@@ -176,9 +131,9 @@ public final class SharedFormulaRecord extends Record {
                                 areaNPtg.isFirstColRelative(),
                                 areaNPtg.isLastColRelative());
             } else {
-            	if (false) {// do we need a ptg clone here?
+                if (false) {// do we need a ptg clone here?
-            		ptg = ptg.copy();
+                    ptg = ptg.copy();
-            	}
+                }
             }
             if (!ptg.isBaseToken()) {
                 ptg.setClass(originalOperandClass);
@@ -194,12 +149,12 @@ public final class SharedFormulaRecord extends Record {
      * counter part
      */
     public void convertSharedFormulaRecord(FormulaRecord formula) {
-      //Sanity checks
+        int formulaRow = formula.getRow();
-        if (!isFormulaInShared(formula)) {
+        int formulaColumn = formula.getColumn();
+        //Sanity checks
+        if (!isInRange(formulaRow, formulaColumn)) {
             throw new RuntimeException("Shared Formula Conversion: Coding Error");
         }
-        final int formulaRow = formula.getRow();
-        final int formulaColumn = formula.getColumn();
 
         Ptg[] ptgs = convertSharedFormulas(field_7_parsed_expr, formulaRow, formulaColumn);
         formula.setParsedExpression(ptgs);
@@ -223,22 +178,6 @@ public final class SharedFormulaRecord extends Record {
         return row;
     }
 
-    /**
-     * Mirroring formula records so it is registered in the ValueRecordsAggregate
-     */
-    public boolean isInValueSection()
-    {
-         return true;
-    }
-
-
-     /**
-      * Register it in the ValueRecordsAggregate so it can go into the FormulaRecordAggregate
-      */
-     public boolean isValue() {
-         return true;
-     }
-
     public Object clone() {
         //Because this record is converted to individual Formula records, this method is not required.
         throw new UnsupportedOperationException("Cannot clone a SharedFormulaRecord");

src/java/org/apache/poi/hssf/record/SharedValueRecordBase.java

@@ -0,0 +1,134 @@
+/* ====================================================================
+   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.record;
+
+import org.apache.poi.hssf.util.CellRangeAddress8Bit;
+import org.apache.poi.util.LittleEndian;
+
+/**
+ * Common base class for {@link SharedFormulaRecord}, {@link ArrayRecord} and
+ * {@link TableRecord} which are have similarities.
+ * 
+ * @author Josh Micich
+ */
+public abstract class SharedValueRecordBase extends Record {
+
+	private CellRangeAddress8Bit _range;
+
+	protected SharedValueRecordBase(CellRangeAddress8Bit range) {
+		_range = range;
+	}
+
+	protected SharedValueRecordBase() {
+		this(new CellRangeAddress8Bit(0, 0, 0, 0));
+	}
+
+	/**
+	 * reads only the range (1 {@link CellRangeAddress8Bit}) from the stream
+	 */
+	public SharedValueRecordBase(RecordInputStream in) {
+		_range = new CellRangeAddress8Bit(in);
+	}
+
+	protected final void validateSid(short id) {
+		if (id != getSid()) {
+			throw new RecordFormatException("Not a valid SharedFormula");
+		}
+	}
+
+	public final CellRangeAddress8Bit getRange() {
+		return _range;
+	}
+
+	public final int getFirstRow() {
+		return _range.getFirstRow();
+	}
+
+	public final int getLastRow() {
+		return _range.getLastRow();
+	}
+
+	public final int getFirstColumn() {
+		return (short) _range.getFirstColumn();
+	}
+
+	public final int getLastColumn() {
+		return (short) _range.getLastColumn();
+	}
+
+	public final int getRecordSize() {
+		return 4 + CellRangeAddress8Bit.ENCODED_SIZE + getExtraDataSize();
+	}
+
+	protected abstract int getExtraDataSize();
+
+	protected abstract void serializeExtraData(int offset, byte[] data);
+
+	public final int serialize(int offset, byte[] data) {
+		int dataSize = CellRangeAddress8Bit.ENCODED_SIZE + getExtraDataSize();
+
+		LittleEndian.putShort(data, 0 + offset, getSid());
+		LittleEndian.putUShort(data, 2 + offset, dataSize);
+
+		int pos = offset + 4;
+		_range.serialize(pos, data);
+		pos += CellRangeAddress8Bit.ENCODED_SIZE;
+		serializeExtraData(pos, data);
+		return dataSize + 4;
+	}
+
+	protected final void fillFields(RecordInputStream in) {
+		throw new RuntimeException("Should not be called.  Fields are filled in constructor");
+	}
+
+	/**
+	 * @return <code>true</code> if (rowIx, colIx) is within the range ({@link #getRange()})
+	 * of this shared value object.
+	 */
+	public final boolean isInRange(int rowIx, int colIx) {
+		CellRangeAddress8Bit r = _range;
+		return r.getFirstRow() <= rowIx 
+			&& r.getLastRow() >= rowIx
+			&& r.getFirstColumn() <= colIx 
+			&& r.getLastColumn() >= colIx;
+	}
+	/**
+	 * @return <code>true</code> if (rowIx, colIx) describes the first cell in this shared value 
+	 * object's range ({@link #getRange()})
+	 */
+	public final boolean isFirstCell(int rowIx, int colIx) {
+		CellRangeAddress8Bit r = getRange();
+		return r.getFirstRow() == rowIx && r.getFirstColumn() == colIx;
+	}
+
+	/**
+	 * Mirroring formula records so it is registered in the
+	 * ValueRecordsAggregate
+	 */
+	public final boolean isInValueSection() {
+		return true;
+	}
+
+	/**
+	 * Register it in the ValueRecordsAggregate so it can go into the
+	 * FormulaRecordAggregate
+	 */
+	public final boolean isValue() {
+		return true;
+	}
+}

src/java/org/apache/poi/hssf/record/TableRecord.java

@@ -34,19 +34,15 @@ import org.apache.poi.util.LittleEndian;
  *
  * See p536 of the June 08 binary docs
  */
-public final class TableRecord extends Record {
+public final class TableRecord extends SharedValueRecordBase {
 	public static final short sid = 0x0236;
 
 	private static final BitField alwaysCalc      = BitFieldFactory.getInstance(0x0001);
-	private static final BitField reserved1       = BitFieldFactory.getInstance(0x0002);
+	private static final BitField calcOnOpen      = BitFieldFactory.getInstance(0x0002);
 	private static final BitField rowOrColInpCell = BitFieldFactory.getInstance(0x0004);
 	private static final BitField oneOrTwoVar     = BitFieldFactory.getInstance(0x0008);
 	private static final BitField rowDeleted      = BitFieldFactory.getInstance(0x0010);
 	private static final BitField colDeleted      = BitFieldFactory.getInstance(0x0020);
-	private static final BitField reserved2       = BitFieldFactory.getInstance(0x0040);
-	private static final BitField reserved3       = BitFieldFactory.getInstance(0x0080);
-
-	private CellRangeAddress8Bit _range;
 
 	private int field_5_flags;
 	private int field_6_res;
@@ -55,9 +51,8 @@ public final class TableRecord extends Record {
 	private int field_9_rowInputCol;
 	private int field_10_colInputCol;
 
-
+	public TableRecord(RecordInputStream in) {
-	protected void fillFields(RecordInputStream in) {
+		super(in);
-		_range = new CellRangeAddress8Bit(in);
 		field_5_flags        = in.readByte();
 		field_6_res          = in.readByte();
 		field_7_rowInputRow  = in.readShort();
@@ -66,18 +61,11 @@ public final class TableRecord extends Record {
 		field_10_colInputCol = in.readShort();
 	}
 
-	public TableRecord(RecordInputStream in) {
-		super(in);
-	}
 	public TableRecord(CellRangeAddress8Bit range) {
-		_range = range;
+		super(range);
 		field_6_res = 0;
 	}
 
-	public CellRangeAddress8Bit getRange() {
-		return _range;
-	}
-
 	public int getFlags() {
 		return field_5_flags;
 	}
@@ -153,43 +141,24 @@ public final class TableRecord extends Record {
 	public short getSid() {
 		return sid;
 	}
-
+	protected int getExtraDataSize() {
-	public int serialize(int offset, byte[] data) {
+		return 
-		int dataSize = getDataSize();
+		2 // 2 byte fields
-		LittleEndian.putShort(data, 0 + offset, sid);
+		+ 8; // 4 short fields
-		LittleEndian.putUShort(data, 2 + offset, dataSize);
-
-		_range.serialize(4 + offset, data);
-		LittleEndian.putByte(data, 10 + offset, field_5_flags);
-		LittleEndian.putByte(data, 11 + offset, field_6_res);
-		LittleEndian.putUShort(data, 12 + offset, field_7_rowInputRow);
-		LittleEndian.putUShort(data, 14 + offset, field_8_colInputRow);
-		LittleEndian.putUShort(data, 16 + offset, field_9_rowInputCol);
-		LittleEndian.putUShort(data, 18 + offset, field_10_colInputCol);
-
-		return 4 + dataSize;
 	}
-	private int getDataSize() {
+	protected void serializeExtraData(int offset, byte[] data) {
-		return CellRangeAddress8Bit.ENCODED_SIZE
+		LittleEndian.putByte(data, 0 + offset, field_5_flags);
-			+ 2 // 2 byte fields
+		LittleEndian.putByte(data, 1 + offset, field_6_res);
-			+ 8; // 4 short fields
+		LittleEndian.putUShort(data, 2 + offset, field_7_rowInputRow);
-	}
+		LittleEndian.putUShort(data, 4 + offset, field_8_colInputRow);
-
+		LittleEndian.putUShort(data, 6 + offset, field_9_rowInputCol);
-	public int getRecordSize() {
+		LittleEndian.putUShort(data, 8 + offset, field_10_colInputCol);
-		return 4+getDataSize();
-	}
-
-	protected void validateSid(short id) {
-		if (id != sid)
-		{
-			throw new RecordFormatException("NOT A TABLE RECORD");
-		}
 	}
 
 	public String toString() {
 		StringBuffer buffer = new StringBuffer();
 		buffer.append("[TABLE]\n");
-		buffer.append("    .range    = ").append(_range.toString()).append("\n");
+		buffer.append("    .range    = ").append(getRange().toString()).append("\n");
 		buffer.append("    .flags    = ") .append(HexDump.byteToHex(field_5_flags)).append("\n");
 		buffer.append("    .alwaysClc= ").append(isAlwaysCalc()).append("\n");
 		buffer.append("    .reserved = ").append(HexDump.intToHex(field_6_res)).append("\n");

src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java

@@ -17,12 +17,10 @@
 
 package org.apache.poi.hssf.record.aggregates;
 
-import org.apache.poi.hssf.model.RecordStream;
 import org.apache.poi.hssf.record.CellValueRecordInterface;
 import org.apache.poi.hssf.record.FormulaRecord;
-import org.apache.poi.hssf.record.SharedFormulaRecord;
+import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.StringRecord;
-import org.apache.poi.hssf.record.TableRecord;
 
 /**
  * The formula record aggregate is used to join together the formula record and it's
@@ -33,35 +31,29 @@ import org.apache.poi.hssf.record.TableRecord;
 public final class FormulaRecordAggregate extends RecordAggregate implements CellValueRecordInterface {
 
     private final FormulaRecord _formulaRecord;
+    private SharedValueManager _sharedValueManager;
     /** caches the calculated result of the formula */
     private StringRecord _stringRecord;
-    private TableRecord _tableRecord;
     
-    public FormulaRecordAggregate(FormulaRecord formulaRecord) {
+    /**
-        _formulaRecord = formulaRecord;
+     * @param stringRec may be <code>null</code> if this formula does not have a cached text 
-        _stringRecord = null;
+     * value.
-    }
+     * @param svm the {@link SharedValueManager} for the current sheet
-    public FormulaRecordAggregate(FormulaRecord formulaRecord, RecordStream rs) {
+     */
-        _formulaRecord = formulaRecord;
+    public FormulaRecordAggregate(FormulaRecord formulaRec, StringRecord stringRec, SharedValueManager svm) {
-        Class nextClass = rs.peekNextClass();
+        if (svm == null) {
-        if (nextClass == SharedFormulaRecord.class) {
+            throw new IllegalArgumentException("sfm must not be null");
-            // For (text) shared formulas, the SharedFormulaRecord comes before the StringRecord.
-            // In any case it is OK to skip SharedFormulaRecords because they were collected 
-            // before constructing the ValueRecordsAggregate.
-            rs.getNext(); // skip the shared formula record
-            nextClass = rs.peekNextClass();
         }
-        if (nextClass == StringRecord.class) {
+        if (formulaRec.isSharedFormula()) {
-            _stringRecord = (StringRecord) rs.getNext();
+            svm.convertSharedFormulaRecord(formulaRec);
-        } else if (nextClass == TableRecord.class) {
-            _tableRecord = (TableRecord) rs.getNext();
         }
+        _formulaRecord = formulaRec;
+        _sharedValueManager = svm;
+        _stringRecord = stringRec;
     }
 
     public void setStringRecord(StringRecord stringRecord) {
         _stringRecord = stringRecord;
-        _tableRecord = null; // probably can't have both present at the same time
-        // TODO - establish rules governing when each of these sub records may exist
     }
     
     public FormulaRecord getFormulaRecord() {
@@ -102,12 +94,13 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel
    
     public void visitContainedRecords(RecordVisitor rv) {
          rv.visitRecord(_formulaRecord);
+         Record sharedFormulaRecord = _sharedValueManager.getRecordForFirstCell(_formulaRecord);
+         if (sharedFormulaRecord != null) {
+             rv.visitRecord(sharedFormulaRecord);
+         }
          if (_stringRecord != null) {
              rv.visitRecord(_stringRecord);
          }
-         if (_tableRecord != null) {
-             rv.visitRecord(_tableRecord);
-        }
     }
    
     public String getStringValue() {

src/java/org/apache/poi/hssf/record/aggregates/MergedCellsTable.java

@@ -93,8 +93,13 @@ public final class MergedCellsTable extends RecordAggregate {
 			rv.visitRecord(new MergeCellsRecord(cras, startIx, nLeftoverMergedRegions));
 		}
 	}
+	public void addRecords(MergeCellsRecord[] mcrs) {
+		for (int i = 0; i < mcrs.length; i++) {
+			addMergeCellsRecord(mcrs[i]);
+		}
+	}
 
-	public void add(MergeCellsRecord mcr) {
+	private void addMergeCellsRecord(MergeCellsRecord mcr) {
 		int nRegions = mcr.getNumAreas();
 		for (int i = 0; i < nRegions; i++) {
 			_mergedRegions.add(mcr.getAreaAt(i));
@@ -125,4 +130,5 @@ public final class MergedCellsTable extends RecordAggregate {
 	public int getNumberOfMergedRegions() {
 		return _mergedRegions.size();
 	}
+
 }

src/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java

@@ -23,12 +23,17 @@ import java.util.List;
 import java.util.Map;
 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.DBCellRecord;
+import org.apache.poi.hssf.record.FormulaRecord;
 import org.apache.poi.hssf.record.IndexRecord;
 import org.apache.poi.hssf.record.MergeCellsRecord;
 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.TableRecord;
 import org.apache.poi.hssf.record.UnknownRecord;
 
 /**
@@ -42,36 +47,34 @@ public final class RowRecordsAggregate extends RecordAggregate {
     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(new TreeMap(), new ValueRecordsAggregate());
+        this(SharedValueManager.EMPTY);
     }
-    private RowRecordsAggregate(TreeMap rowRecords, ValueRecordsAggregate valuesAgg) {
+    private RowRecordsAggregate(SharedValueManager svm) {
-        _rowRecords = rowRecords;
+        _rowRecords = new TreeMap();
-        _valuesAgg = valuesAgg;
+        _valuesAgg = new ValueRecordsAggregate();
         _unknownRecords = new ArrayList();
+        _sharedValueManager = svm;
     }
 
-    public RowRecordsAggregate(List recs, int startIx, int endIx) {
+    /**
-        this();
+     * @param rs record stream with all {@link SharedFormulaRecord}
-        // First up, locate all the shared formulas for this sheet
+     * {@link ArrayRecord}, {@link TableRecord} {@link MergeCellsRecord} Records removed
-        SharedFormulaHolder sfh = SharedFormulaHolder.create(recs, startIx, endIx);
+     */
-        for(int i=startIx; i<endIx; i++) {
+    public RowRecordsAggregate(RecordStream rs, SharedValueManager svm) {
-            Record rec = (Record) recs.get(i);
+        this(svm);
+        while(rs.hasNext()) {
+            Record rec = rs.getNext();
             switch (rec.getSid()) {
-                case MergeCellsRecord.sid:
-                    // Some apps scatter these records between the rows/cells but they are supposed to
-                    // be well after the row/cell records.  It is assumed such rogue MergeCellRecords 
-                    // have already been collected by the caller, and can safely be ignored here. 
-                    // see bug 45699
-                    continue;
                 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) {
@@ -82,9 +85,8 @@ public final class RowRecordsAggregate extends RecordAggregate {
             if (!rec.isValue()) {
                 throw new RuntimeException("Unexpected record type (" + rec.getClass().getName() + ")");
             }
-            i += _valuesAgg.construct(recs, i, endIx, sfh)-1;
+            _valuesAgg.construct((CellValueRecordInterface)rec, rs, svm);
         }
-        "".length();
     }
     /**
      * Handles UnknownRecords which appear within the row/cell records
@@ -230,11 +232,11 @@ public final class RowRecordsAggregate extends RecordAggregate {
           rv.visitRecord(rec);
         }
         return result;
-      }
+    }
 
     public void visitContainedRecords(RecordVisitor rv) {
-        ValueRecordsAggregate cells = _valuesAgg;
 
+        PositionTrackingVisitor stv = new PositionTrackingVisitor(rv, 0);
         //DBCells are serialized before row records.
         final int blockCount = getRowBlockCount();
         for (int blockIndex = 0; blockIndex < blockCount; blockIndex++) {
@@ -251,8 +253,10 @@ public final class RowRecordsAggregate extends RecordAggregate {
             // Note: Cell references start from the second row...
             int cellRefOffset = (rowBlockSize - RowRecord.ENCODED_SIZE);
             for (int row = startRowNumber; row <= endRowNumber; row++) {
-                if (cells.rowHasCells(row)) {
+                if (_valuesAgg.rowHasCells(row)) {
-                    final int rowCellSize = cells.visitCellsForRow(row, rv);
+                    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.
@@ -274,7 +278,6 @@ public final class RowRecordsAggregate extends RecordAggregate {
         return _rowRecords.values().iterator();
     }
 
-    
     public Iterator getAllRecordsIterator() {
         List result = new ArrayList(_rowRecords.size() * 2);
         result.addAll(_rowRecords.values());
@@ -498,5 +501,10 @@ public final class RowRecordsAggregate extends RecordAggregate {
     public void removeCell(CellValueRecordInterface cvRec) {
         _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);
+    }
 }
-

src/java/org/apache/poi/hssf/record/aggregates/SharedFormulaHolder.java

@@ -1,96 +0,0 @@
-/* ====================================================================
-   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.record.aggregates;
-
-import java.util.ArrayList;
-import java.util.List;
-
-import org.apache.poi.hssf.record.FormulaRecord;
-import org.apache.poi.hssf.record.Record;
-import org.apache.poi.hssf.record.SharedFormulaRecord;
-
-/**
- * Temporarily holds SharedFormulaRecords while constructing a <tt>RowRecordsAggregate</tt>
- * 
- * @author Josh Micich
- */
-final class SharedFormulaHolder {
-
-	private static final SharedFormulaHolder EMPTY = new SharedFormulaHolder(new SharedFormulaRecord[0]);
-	private final SharedFormulaRecord[] _sfrs;
-
-	/**
-	 * @param recs list of sheet records (possibly contains records for other parts of the Excel file)
-	 * @param startIx index of first row/cell record for current sheet
-	 * @param endIx one past index of last row/cell record for current sheet.  It is important 
-	 * that this code does not inadvertently collect <tt>SharedFormulaRecord</tt>s from any other
-	 * sheet (which could happen if endIx is chosen poorly).  (see bug 44449) 
-	 */
-	public static SharedFormulaHolder create(List recs, int startIx, int endIx) {
-		List temp = new ArrayList();
-        for (int k = startIx; k < endIx; k++)
-        {
-            Record rec = ( Record ) recs.get(k);
-            if (rec instanceof SharedFormulaRecord) {
-                temp.add(rec);
-            }
-        }
-        if (temp.size() < 1) {
-        	return EMPTY;
-        }
-        SharedFormulaRecord[] sfrs = new SharedFormulaRecord[temp.size()];
-        temp.toArray(sfrs);
-        return new SharedFormulaHolder(sfrs);
-        
-	}
-	private SharedFormulaHolder(SharedFormulaRecord[] sfrs) {
-		_sfrs = sfrs;
-	}
-	public void convertSharedFormulaRecord(FormulaRecord formula) {
-        // Traverse the list of shared formulas in
-        //  reverse order, and try to find the correct one
-        //  for us
-        for (int i=0; i<_sfrs.length; i++) {
-            SharedFormulaRecord shrd = _sfrs[i];
-            if (shrd.isFormulaInShared(formula)) {
-                shrd.convertSharedFormulaRecord(formula);
-                return;
-            }
-        }
-        // not found
-        handleMissingSharedFormulaRecord(formula);
-	}
-
-    /**
-     * Sometimes the shared formula flag "seems" to be erroneously set, in which case there is no 
-     * call to <tt>SharedFormulaRecord.convertSharedFormulaRecord</tt> and hence the 
-     * <tt>parsedExpression</tt> field of this <tt>FormulaRecord</tt> will not get updated.<br/>
-     * As it turns out, this is not a problem, because in these circumstances, the existing value
-     * for <tt>parsedExpression</tt> is perfectly OK.<p/>
-     * 
-     * This method may also be used for setting breakpoints to help diagnose issues regarding the
-     * abnormally-set 'shared formula' flags. 
-     * (see TestValueRecordsAggregate.testSpuriousSharedFormulaFlag()).<p/>
-     * 
-     * The method currently does nothing but do not delete it without finding a nice home for this 
-     * comment.
-     */
-    private static void handleMissingSharedFormulaRecord(FormulaRecord formula) {
-        // could log an info message here since this is a fairly unusual occurrence.
-    }
-}

src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java

@@ -0,0 +1,130 @@
+/* ====================================================================
+   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.record.aggregates;
+
+import org.apache.poi.hssf.record.ArrayRecord;
+import org.apache.poi.hssf.record.FormulaRecord;
+import org.apache.poi.hssf.record.SharedFormulaRecord;
+import org.apache.poi.hssf.record.SharedValueRecordBase;
+import org.apache.poi.hssf.record.TableRecord;
+
+/**
+ * Manages various auxiliary records while constructing a
+ * {@link RowRecordsAggregate}:
+ * <ul>
+ * <li>{@link SharedFormulaRecord}s</li>
+ * <li>{@link ArrayRecord}s</li>
+ * <li>{@link TableRecord}s</li>
+ * </ul>
+ * 
+ * @author Josh Micich
+ */
+public final class SharedValueManager {
+
+	public static final SharedValueManager EMPTY = new SharedValueManager(
+			new SharedFormulaRecord[0], new ArrayRecord[0], new TableRecord[0]);
+	private final SharedFormulaRecord[] _sfrs;
+	private final ArrayRecord[] _arrayRecords;
+	private final TableRecord[] _tableRecords;
+
+	private SharedValueManager(SharedFormulaRecord[] sharedFormulaRecords,
+			ArrayRecord[] arrayRecords, TableRecord[] tableRecords) {
+		_sfrs = sharedFormulaRecords;
+		_arrayRecords = arrayRecords;
+		_tableRecords = tableRecords;
+	}
+
+	/**
+	 * @param recs list of sheet records (possibly contains records for other parts of the Excel file)
+	 * @param startIx index of first row/cell record for current sheet
+	 * @param endIx one past index of last row/cell record for current sheet.  It is important 
+	 * that this code does not inadvertently collect <tt>SharedFormulaRecord</tt>s from any other
+	 * sheet (which could happen if endIx is chosen poorly).  (see bug 44449) 
+	 */
+	public static SharedValueManager create(SharedFormulaRecord[] sharedFormulaRecords,
+			ArrayRecord[] arrayRecords, TableRecord[] tableRecords) {
+		if (sharedFormulaRecords.length + arrayRecords.length + tableRecords.length < 1) {
+			return EMPTY;
+		}
+		return new SharedValueManager(sharedFormulaRecords, arrayRecords, tableRecords);
+	}
+
+	public void convertSharedFormulaRecord(FormulaRecord formula) {
+		int row = formula.getRow();
+		int column = formula.getColumn();
+		// Traverse the list of shared formulas in
+		// reverse order, and try to find the correct one
+		// for us
+		for (int i = 0; i < _sfrs.length; i++) {
+			SharedFormulaRecord shrd = _sfrs[i];
+			if (shrd.isInRange(row, column)) {
+				shrd.convertSharedFormulaRecord(formula);
+				return;
+			}
+		}
+		// not found
+		handleMissingSharedFormulaRecord(formula);
+	}
+
+	/**
+	 * Sometimes the shared formula flag "seems" to be erroneously set, in which case there is no 
+	 * call to <tt>SharedFormulaRecord.convertSharedFormulaRecord</tt> and hence the 
+	 * <tt>parsedExpression</tt> field of this <tt>FormulaRecord</tt> will not get updated.<br/>
+	 * As it turns out, this is not a problem, because in these circumstances, the existing value
+	 * for <tt>parsedExpression</tt> is perfectly OK.<p/>
+	 * 
+	 * This method may also be used for setting breakpoints to help diagnose issues regarding the
+	 * abnormally-set 'shared formula' flags. 
+	 * (see TestValueRecordsAggregate.testSpuriousSharedFormulaFlag()).<p/>
+	 * 
+	 * The method currently does nothing but do not delete it without finding a nice home for this 
+	 * comment.
+	 */
+	private static void handleMissingSharedFormulaRecord(FormulaRecord formula) {
+		// could log an info message here since this is a fairly unusual occurrence.
+		formula.setSharedFormula(false); // no point leaving the flag erroneously set
+	}
+
+	/**
+	 * Note - does not return SharedFormulaRecords currently, because the corresponding formula
+	 * records have been converted to 'unshared'. POI does not attempt to re-share formulas. On
+	 * the other hand, there is no such conversion for array or table formulas, so this method 
+	 * returns the TABLE or ARRAY record (if it should be written after the specified 
+	 * formulaRecord.
+	 * 
+	 * @return the TABLE or ARRAY record for this formula cell, if it is the first cell of a 
+	 * table or array region.
+	 */
+	public SharedValueRecordBase getRecordForFirstCell(FormulaRecord formulaRecord) {
+		int row = formulaRecord.getRow();
+		int column = formulaRecord.getColumn();
+		for (int i = 0; i < _tableRecords.length; i++) {
+			TableRecord tr = _tableRecords[i];
+			if (tr.isFirstCell(row, column)) {
+				return tr;
+			}
+		}
+		for (int i = 0; i < _arrayRecords.length; i++) {
+			ArrayRecord ar = _arrayRecords[i];
+			if (ar.isFirstCell(row, column)) {
+				return ar;
+			}
+		}
+		return null;
+	}
+}

src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java

@@ -23,16 +23,10 @@ import java.util.List;
 
 import org.apache.poi.hssf.model.RecordStream;
 import org.apache.poi.hssf.record.CellValueRecordInterface;
-import org.apache.poi.hssf.record.DBCellRecord;
 import org.apache.poi.hssf.record.FormulaRecord;
-import org.apache.poi.hssf.record.MergeCellsRecord;
 import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.RecordBase;
-import org.apache.poi.hssf.record.RowRecord;
-import org.apache.poi.hssf.record.SharedFormulaRecord;
 import org.apache.poi.hssf.record.StringRecord;
-import org.apache.poi.hssf.record.TableRecord;
-import org.apache.poi.hssf.record.UnknownRecord;
 import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
 
 /**
@@ -143,63 +137,27 @@ public final class ValueRecordsAggregate {
     }
 
     /**
-     * Processes a sequential group of cell value records.  Stops at endIx or the first
+     * Processes a single cell value record
-     * non-value record encountered.
+     * @param sfh used to resolve any shared-formulas/arrays/tables for the current sheet
-     * @param sfh used to resolve any shared formulas for the current sheet
-     * @return the number of records consumed
      */
-    public int construct(List records, int offset, int endIx, SharedFormulaHolder sfh) {
+    public void construct(CellValueRecordInterface rec, RecordStream rs, SharedValueManager sfh) {
-        RecordStream rs = new RecordStream(records, offset, endIx);
+        if (rec instanceof FormulaRecord) {
-
+            FormulaRecord formulaRec = (FormulaRecord)rec;
-        // Now do the main processing sweep
+            if (formulaRec.isSharedFormula()) {
-        while (rs.hasNext()) {
+                sfh.convertSharedFormulaRecord(formulaRec);
-            Class recClass = rs.peekNextClass();
-            if (recClass == StringRecord.class) {
-                throw new RuntimeException("Loose StringRecord found without preceding FormulaRecord");
             }
-
+            // read optional cached text value
-            if (recClass == TableRecord.class) {
+            StringRecord cachedText;
-                throw new RuntimeException("Loose TableRecord found without preceding FormulaRecord");
+            Class nextClass = rs.peekNextClass();
+            if (nextClass == StringRecord.class) {
+                cachedText = (StringRecord) rs.getNext();
+            } else {
+                cachedText = null;
             }
-
+            insertCell(new FormulaRecordAggregate(formulaRec, cachedText, sfh));
-            if (recClass == UnknownRecord.class) {
+        } else {
-                break;
+            insertCell(rec);
-            }
-            if (recClass == RowRecord.class) {
-                break;
-            }
-            if (recClass == DBCellRecord.class) {
-                // end of 'Row Block'.  This record is ignored by POI
-                break;
-            }
-
-            Record rec = rs.getNext();
-
-            if (recClass == SharedFormulaRecord.class) {
-                // Already handled, not to worry
-                continue;
-            }
-            if (recClass == MergeCellsRecord.class) {
-                // doesn't really belong here
-                // can safely be ignored, because it has been processed in a higher method
-                continue;
-            }
-
-            if (!rec.isValue()) {
-                throw new RuntimeException("bad record type");
-            }
-            if (rec instanceof FormulaRecord) {
-                FormulaRecord formula = (FormulaRecord)rec;
-                if (formula.isSharedFormula()) {
-                    sfh.convertSharedFormulaRecord(formula);
-                }
-
-                insertCell(new FormulaRecordAggregate((FormulaRecord)rec, rs));
-                continue;
-            }
-            insertCell(( CellValueRecordInterface ) rec);
         }
-        return rs.getCountRead();
     }
 
     /** Tallies a count of the size of the cell records
@@ -247,8 +205,8 @@ public final class ValueRecordsAggregate {
         return pos - offset;
     }
 
-    public int visitCellsForRow(int rowIndex, RecordVisitor rv) {
+    public void visitCellsForRow(int rowIndex, RecordVisitor rv) {
-        int result = 0;
+
         CellValueRecordInterface[] cellRecs = records[rowIndex];
         if (cellRecs != null) {
             for (int i = 0; i < cellRecs.length; i++) {
@@ -256,24 +214,15 @@ public final class ValueRecordsAggregate {
                 if (cvr == null) {
                     continue;
                 }
-                if (cvr instanceof FormulaRecordAggregate) {
+                if (cvr instanceof RecordAggregate) {
-                    FormulaRecordAggregate fmAgg = (FormulaRecordAggregate) cvr;
+                    RecordAggregate agg = (RecordAggregate) cvr;
-                    Record fmAggRec = fmAgg.getFormulaRecord();
+                    agg.visitContainedRecords(rv);
-                    rv.visitRecord(fmAggRec);
-                    result += fmAggRec.getRecordSize();
-                    fmAggRec = fmAgg.getStringRecord();
-                    if (fmAggRec != null) {
-                        rv.visitRecord(fmAggRec);
-                        result += fmAggRec.getRecordSize();
-                    }
                 } else {
                     Record rec = (Record) cvr;
                     rv.visitRecord(rec);
-                    result += rec.getRecordSize();
                 }
             }
         }
-        return result;
     }
 
     public CellValueRecordInterface[] getValueRecords() {

src/java/org/apache/poi/hssf/usermodel/HSSFCell.java

@@ -288,23 +288,20 @@ public final class HSSFCell {
         {
 
             case CELL_TYPE_FORMULA :
-                FormulaRecordAggregate frec = null;
+                FormulaRecordAggregate frec;
 
-                if (cellType != this.cellType)
+                if (cellType != this.cellType) {
-                {
+                    frec = sheet.createFormula(row, col);
-                    frec = new FormulaRecordAggregate(new FormulaRecord());
+                } else {
+                    frec = (FormulaRecordAggregate) record;
+                    frec.setRow(row);
+                    frec.setColumn(col);
                 }
-                else
-                {
-                    frec = ( FormulaRecordAggregate ) record;
-                }
-                frec.setColumn(col);
                 if (setValue)
                 {
                     frec.getFormulaRecord().setValue(getNumericCellValue());
                 }
                 frec.setXFIndex(styleIndex);
-                frec.setRow(row);
                 record = frec;
                 break;
 

src/testcases/org/apache/poi/hssf/record/aggregates/TestFormulaRecordAggregate.java

@@ -15,13 +15,10 @@
    limitations under the License.
 ==================================================================== */
 
-/*
- * TestFormulaRecordAggregate.java
- *
- * Created on March 21, 2003, 12:32 AM
- */
-
 package org.apache.poi.hssf.record.aggregates;
+
+import junit.framework.TestCase;
+
 import org.apache.poi.hssf.record.FormulaRecord;
 import org.apache.poi.hssf.record.StringRecord;
 
@@ -29,14 +26,13 @@ import org.apache.poi.hssf.record.StringRecord;
  *
  * @author  avik
  */
-public final class TestFormulaRecordAggregate extends junit.framework.TestCase {
+public final class TestFormulaRecordAggregate extends TestCase {
     
     public void testBasic() throws Exception {
         FormulaRecord f = new FormulaRecord();
         StringRecord s = new StringRecord();
         s.setString("abc");
-        FormulaRecordAggregate fagg = new FormulaRecordAggregate(f);
+        FormulaRecordAggregate fagg = new FormulaRecordAggregate(f, s, SharedValueManager.EMPTY);
-        fagg.setStringRecord(s);
         assertEquals("abc", fagg.getStringValue());
     }
 }

src/testcases/org/apache/poi/hssf/record/aggregates/TestRowRecordsAggregate.java

@@ -17,25 +17,100 @@
 
 package org.apache.poi.hssf.record.aggregates;
 
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+
+import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
+import org.apache.poi.hssf.HSSFTestDataSamples;
+import org.apache.poi.hssf.record.ArrayRecord;
+import org.apache.poi.hssf.record.FormulaRecord;
+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.usermodel.HSSFWorkbook;
+import org.apache.poi.hssf.usermodel.RecordInspector;
+import org.apache.poi.hssf.util.CellRangeAddress8Bit;
 
 /**
  * 
  */
 public final class TestRowRecordsAggregate extends TestCase {
 
-    public void testRowGet() {
+	public void testRowGet() {
-        RowRecordsAggregate rra = new RowRecordsAggregate();
+		RowRecordsAggregate rra = new RowRecordsAggregate();
-        RowRecord rr = new RowRecord(4);
+		RowRecord rr = new RowRecord(4);
-        rra.insertRow(rr);
+		rra.insertRow(rr);
-        rra.insertRow(new RowRecord(1));
+		rra.insertRow(new RowRecord(1));
 
-        RowRecord rr1 = rra.getRow(4);
+		RowRecord rr1 = rra.getRow(4);
 
-        assertNotNull(rr1);
+		assertNotNull(rr1);
-        assertEquals("Row number is 1", 4, rr1.getRowNumber());
+		assertEquals("Row number is 1", 4, rr1.getRowNumber());
-        assertTrue("Row record retrieved is identical ", rr1 == rr);
+		assertTrue("Row record retrieved is identical ", rr1 == rr);
-    }
+	}
+
+	/**
+	 * Prior to Aug 2008, POI would re-serialize spreadsheets with {@link ArrayRecord}s or
+	 * {@link TableRecord}s with those records out of order.  Similar to 
+	 * {@link SharedFormulaRecord}s, these records should appear immediately after the first
+	 * {@link FormulaRecord}s that they apply to (and only once).<br/>
+	 */
+	public void testArraysAndTables() {
+		HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("testArraysAndTables.xls");
+		Record[] sheetRecs = RecordInspector.getRecords(wb.getSheetAt(0), 0);
+		
+		int countArrayFormulas = verifySharedValues(sheetRecs, ArrayRecord.class);
+		assertEquals(5, countArrayFormulas);
+		int countTableFormulas = verifySharedValues(sheetRecs, TableRecord.class);
+		assertEquals(3, countTableFormulas);
+
+		// Note - SharedFormulaRecords are currently not re-serialized by POI (each is extracted
+		// into many non-shared formulas), but if they ever were, the same rules would apply. 
+		int countSharedFormulas = verifySharedValues(sheetRecs, SharedFormulaRecord.class);
+		assertEquals(0, countSharedFormulas);
+		
+
+		if (false) { // set true to observe re-serialized file
+			File f = new File(System.getProperty("java.io.tmpdir") + "/testArraysAndTables-out.xls");
+			try {
+				OutputStream os = new FileOutputStream(f);
+				wb.write(os);
+				os.close();
+			} catch (IOException e) {
+				throw new RuntimeException(e);
+			}
+			System.out.println("Output file to " + f.getAbsolutePath());
+		}
+	}
+
+	private static int verifySharedValues(Record[] recs, Class shfClass) {
+		
+		int result =0;
+		for(int i=0; i<recs.length; i++) {
+			Record rec = recs[i];
+			if (rec.getClass() == shfClass) {
+				result++;
+				Record prevRec = recs[i-1];
+				if (!(prevRec instanceof FormulaRecord)) {
+					throw new AssertionFailedError("Bad record order at index "
+							+ i + ": Formula record expected but got (" 
+							+ prevRec.getClass().getName() + ")");
+				}
+				verifySharedFormula((FormulaRecord) prevRec, rec);
+			}
+		}
+		return result;
+	}
+
+	private static void verifySharedFormula(FormulaRecord firstFormula, Record rec) {
+		CellRangeAddress8Bit range = ((SharedValueRecordBase)rec).getRange();
+		assertEquals(range.getFirstRow(), firstFormula.getRow());
+		assertEquals(range.getFirstColumn(), firstFormula.getColumn());
+	}
 }

src/testcases/org/apache/poi/hssf/record/aggregates/TestValueRecordsAggregate.java

@@ -28,10 +28,15 @@ 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.model.RowBlocksReader;
 import org.apache.poi.hssf.record.BlankRecord;
+import org.apache.poi.hssf.record.CellValueRecordInterface;
 import org.apache.poi.hssf.record.FormulaRecord;
+import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.RecordBase;
 import org.apache.poi.hssf.record.SharedFormulaRecord;
+import org.apache.poi.hssf.record.WindowTwoRecord;
 import org.apache.poi.hssf.usermodel.HSSFSheet;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 
@@ -47,6 +52,7 @@ public final class TestValueRecordsAggregate extends TestCase {
         List records = new ArrayList();
         records.add( new FormulaRecord() );
         records.add( new SharedFormulaRecord() );
+        records.add(new WindowTwoRecord());
 
         constructValueRecord(records);
         Iterator iterator = valueRecord.getIterator();
@@ -59,8 +65,13 @@ public final class TestValueRecordsAggregate extends TestCase {
     }
 
     private void constructValueRecord(List records) {
-        SharedFormulaHolder sfrh = SharedFormulaHolder.create(records, 0, records.size());
+        RowBlocksReader rbr = new RowBlocksReader(records, 0);
-        valueRecord.construct(records, 0, records.size(), sfrh );
+        SharedValueManager sfrh = rbr.getSharedFormulaManager();
+        RecordStream rs = rbr.getPlainRecordStream();
+        while(rs.hasNext()) {
+            Record rec = rs.getNext();
+            valueRecord.construct((CellValueRecordInterface)rec, rs, sfrh);
+        }
     }
 
     private static List testData() {
@@ -73,6 +84,7 @@ public final class TestValueRecordsAggregate extends TestCase {
         blankRecord.setColumn( (short) 2 );
         records.add( formulaRecord );
         records.add( blankRecord );
+        records.add(new WindowTwoRecord());
         return records;
     }
 

src/testcases/org/apache/poi/hssf/usermodel/RecordInspector.java

@@ -0,0 +1,67 @@
+/* ====================================================================
+   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.usermodel;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
+
+/**
+ * Test utility class to get {@link Record}s out HSSF objects
+ * 
+ * @author Josh Micich
+ */
+public final class RecordInspector {
+
+	private RecordInspector() {
+		// no instances of this class
+	}
+
+	private static final class RecordCollector implements RecordVisitor {
+
+		private List _list;
+
+		public RecordCollector() {
+			_list = new ArrayList(128);
+		}
+
+		public void visitRecord(Record r) {
+			_list.add(r);
+		}
+
+		public Record[] getRecords() {
+			Record[] result = new Record[_list.size()];
+			_list.toArray(result);
+			return result;
+		}
+	}
+
+	/**
+	 * @param streamOffset start position for serialization. This affects values in some
+	 *         records such as INDEX, but most callers will be OK to pass zero.
+	 * @return the {@link Record}s (in order) which will be output when the
+	 *         specified sheet is serialized
+	 */
+	public static Record[] getRecords(HSSFSheet hSheet, int streamOffset) {
+		RecordCollector rc = new RecordCollector();
+		hSheet.getSheet().visitContainedRecords(rc, streamOffset);
+		return rc.getRecords();
+	}
+}