Merged revisions 638786-638802,638805-638811,638813-638814,638816-639230,639233-639241,639243-639253,639255-639486,639488-639601,639603-639835,639837-639917,639919-640056,640058-640710,640712-641156,641158-641184,641186-641795,641797-641798,641800-641933,641935-641963,641965-641966,641968-641995,641997-642230,642232-642562,642564-642565,642568-642570,642572-642573,642576-642736,642739-642877,642879,642881-642890,642892-642903,642905-642945,642947-643624,643626-643653,643655-643669,643671,643673-643830,643832-643833,643835-644342,644344-644472,644474-644508,644510-645347,645349-645351,645353-645559,645561-645565,645568-645951,645953-646193,646195-646311,646313-646404,646406-646665,646667-646853,646855-646869,646871-647151,647153-647185,647187-647277,647279-647566,647568-647573,647575,647578-647711,647714-647737,647739-647823,647825-648155,648157-648202,648204-648273,648275,648277-648302,648304-648333,648335-648588,648590-648622,648625-648673,648675-649141,649144,649146-649556,649558-649795,649799,649801-649910,649912-649913,649915-650128,650131-650132,650134-650137,650140-650914,650916-651991,651993-652284,652286-652287,652289,652291,652293-652297,652299-652328,652330-652425,652427-652445,652447-652560,652562-652933,652935,652937-652993,652995-653116,653118-653124,653126-653483,653487-653519,653522-653550,653552-653607,653609-653667,653669-653674,653676-653814,653817-653830,653832-653891,653893-653944,653946-654055,654057-654355,654357-654365,654367-654648,654651-655215,655217-655277,655279-655281,655283-655911,655913-656212,656214,656216-656251,656253-656698,656700-656756,656758-656892,656894-657135,657137-657165,657168-657179,657181-657354,657356-657357,657359-657701,657703-657874,657876-658032,658034-658284,658286,658288-658301,658303-658307,658309-658321,658323-658335,658337-658348,658351,658353-658832,658834-658983,658985,658987-659066,659068-659402,659404-659428,659430-659451,659453-659454,659456-659461,659463-659477,659479-659524,659526-659571,659574,659576-660255,660257-660262,660264-660279,660281-660343,660345-660473,660475-660827,660829-660833,660835-660888,660890-663321,663323-663435,663437-663764,663766-663854,663856-664219,664221-664489,664494-664514,664516-668013,668015-668142,668144-668152,668154,668156-668256,668258,668260-669139,669141-669455,669457-669657,669659-669808,669810-670189,670191-671321,671323-672229,672231-672549,672551-672552,672554-672561,672563-672566,672568,672571-673049,673051-673852,673854-673862,673864-673986,673988-673996,673998-674347,674349-674890,674892-674910,674912-674936,674938-674952,674954-675078,675080-675085,675087-675217,675219-675660,675662-675670,675672-675716,675718-675726,675728-675733,675735-675775,675777-675782,675784,675786-675791,675794-675852,675854-676200,676202,676204,676206-676220,676222-676309,676311-676456,676458-676994,676996-677027,677030-677040,677042-677056,677058-677375,677377-677968,677970-677971,677973,677975-677994,677996-678286,678288-678538,678540-680393,680395-680469,680471-680529,680531-680852,680854-681529,681531-681571,681573-682224,682226,682228,682231-682281,682283-682335,682337-682507,682509,682512-682517,682519-682532,682534-682619,682622-682777,682779-682998,683000-683019,683021-683022,683024-683080,683082-683092,683094-683095,683097-683127,683129-683131,683133-683166,683168-683698,683700-683705,683707-683757,683759-683787,683789-683870,683872-683879,683881-683900,683902-684066,684068-684074,684076-684222,684224-684254,684257-684281,684283-684286,684288-684292,684294-684298,684300-684301,684303-684308,684310-684317,684320,684323-684335,684337-684348,684350-684354,684356-684361,684363-684369,684371-684453,684455-684883,684885-684937,684940-684958,684960-684970,684972-684985,684987-685053,685055-685063,685065-685259,685261-685262,685264-685266,685268-685282,685285-686035,686037-686045,686047-686052,686054-686206,686208-686215,686217-686277,686279-686289,686291-686620,686622-686623,686626-686627,686629-686639,686641-686843,686845-686976,686978-687402,687404-687422,687424-687428,687430-687442,687444-688425,688427-688641,688643-688649,688651-688654,688656-688824,688826-688909,688911-689543,689545-689558,689560-689635,689637-689703,689705-689715,689717-689718,689720,689722-689972,689974-690090,690092-690093,690095-690111,690113-690258,690260-690261,690263-690403,690405-690410,690412-690460,690462-690516,690518-690730 via svnmerge from

https://svn.apache.org/repos/asf/poi/trunk

........
  r690534 | nick | 2008-08-30 17:59:55 +0100 (Sat, 30 Aug 2008) | 1 line
  
  Start to support HPBF PLC parts
........
  r690536 | nick | 2008-08-30 18:12:42 +0100 (Sat, 30 Aug 2008) | 1 line
  
  Further HPBF plc tests
........
  r690626 | josh | 2008-08-31 02:53:47 +0100 (Sun, 31 Aug 2008) | 1 line
  
  changed serialize method on Sheet to visitContainedRecords to simplify serialization logic and also allow test code to inspect generated sheet records more directly
........
  r690636 | josh | 2008-08-31 05:45:00 +0100 (Sun, 31 Aug 2008) | 1 line
  
  Fix for bugs 26321 and 44958 - preserve position of ArrayRecords and TableRecords among cell value records
........
  r690721 | josh | 2008-08-31 17:27:35 +0100 (Sun, 31 Aug 2008) | 1 line
  
  Added junit to show bug 45717 is fixed. (Previously fixed by either of r683758, r683871)
........
  r690726 | nick | 2008-08-31 17:37:39 +0100 (Sun, 31 Aug 2008) | 1 line
  
  Start to support HPBF hyperlinks
........
  r690729 | nick | 2008-08-31 17:58:29 +0100 (Sun, 31 Aug 2008) | 1 line
  
  Add HPBF hyperlinks support to the extractor
........


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

src/documentation/content/xdocs/changes.xml

@@ -64,6 +64,8 @@
            <action dev="POI-DEVELOPERS" type="add">Created a common interface for handling Excel files, irrespective of if they are .xls or .xlsx</action>
         </release>
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="add">Support for HPBF Publisher hyperlinks, including during text extraction</action>
+           <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

@@ -61,6 +61,8 @@
            <action dev="POI-DEVELOPERS" type="add">Created a common interface for handling Excel files, irrespective of if they are .xls or .xlsx</action>
         </release>
         <release version="3.1.1-alpha1" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="add">Support for HPBF Publisher hyperlinks, including during text extraction</action>
+           <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,10 +62,12 @@ 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;
 import org.apache.poi.hssf.record.aggregates.RowRecordsAggregate;
+import org.apache.poi.hssf.record.aggregates.RecordAggregate.PositionTrackingVisitor;
 import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
 import org.apache.poi.hssf.util.CellRangeAddress;
 import org.apache.poi.hssf.util.PaneInformation;
@@ -105,7 +105,6 @@ public final class Sheet implements Model {
     private static POILogger            log              = POILogFactory.getLogger(Sheet.class);
 
     protected ArrayList                  records           =     null;
-              int                        preoffset         =     0;            // offset of the sheet in a new file
     protected int                        dimsloc           =     -1;  // TODO - remove dimsloc
     protected PrintGridlinesRecord       printGridlines    =     null;
     protected GridsetRecord              gridset           =     null;
@@ -148,7 +147,7 @@ public final class Sheet implements Model {
      * @see #createSheet(List,int,int)
      */
     public Sheet() {
-    	_mergedCellsTable = new MergedCellsTable();
+        _mergedCellsTable = new MergedCellsTable();
     }
 
     /**
@@ -181,18 +180,12 @@ 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);
                 retval.condFormatting = new ConditionalFormattingTable(rs);
@@ -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;
             }
              
@@ -245,13 +239,18 @@ public final class Sheet implements Model {
             }
             
             if (rec.getSid() == MergeCellsRecord.sid) {
-            	// when the MergedCellsTable is found in the right place, we expect those records to be contiguous
+                // when the MergedCellsTable is found in the right place, we expect those records to be contiguous
                 RecordStream rs = new RecordStream(inRecs, k);
                 retval._mergedCellsTable.read(rs);
                 k += rs.getCountRead()-1;
                 continue;
             }
-            
+            if (rec.getSid() == UncalcedRecord.sid) {
+                // don't add UncalcedRecord to the list
+                retval._isUncalced = true; // this flag is enough
+                continue;
+            }
+
             if (rec.getSid() == BOFRecord.sid)
             {
                 bofEofNestingLevel++;
@@ -269,9 +268,6 @@ public final class Sheet implements Model {
                     break;
                 }
             }
-            else if (rec.getSid() == UncalcedRecord.sid) {
-                retval._isUncalced = true;
-            }
             else if (rec.getSid() == DimensionsRecord.sid)
             {
                 // Make a columns aggregate if one hasn't ready been created.
@@ -342,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;
@@ -585,61 +561,23 @@ public final class Sheet implements Model {
             log.log(POILogger.DEBUG, "Sheet.setDimensions exiting");
     }
 
-    /**
+    public void visitContainedRecords(RecordVisitor rv, int offset) {
-     * Set the preoffset when using DBCELL records (currently unused) - this is
-     * the position of this sheet within the whole file.
-     *
-     * @param offset the offset of the sheet's BOF within the file.
-     */
 
-    public void setPreOffset(int offset)
+        PositionTrackingVisitor ptv = new PositionTrackingVisitor(rv, offset);
-    {
+        
-        this.preoffset = offset;
-    }
-
-    /**
-     * get the preoffset when using DBCELL records (currently unused) - this is
-     * the position of this sheet within the whole file.
-     *
-     * @return offset the offset of the sheet's BOF within the file.
-     */
-
-    public int getPreOffset()
-    {
-        return preoffset;
-    }
-
-    /**
-     * Serializes all records in the sheet into one big byte array.  Use this to write
-     * the sheet out.
-     *
-     * @param offset to begin write at
-     * @param data   array containing the binary representation of the records in this sheet
-     *
-     */
-
-    public int serialize(int offset, byte [] data)
-    {
-        if (log.check( POILogger.DEBUG ))
-            log.log(POILogger.DEBUG, "Sheet.serialize using offsets");
-
-        int pos       = offset;
         boolean haveSerializedIndex = false;
 
         for (int k = 0; k < records.size(); k++)
         {
             RecordBase record = (RecordBase) records.get(k);
 
-            // Don't write out UncalcedRecord entries, as
+            if (record instanceof RecordAggregate) {
-            //  we handle those specially just below
+                RecordAggregate agg = (RecordAggregate) record;
-            if (record instanceof UncalcedRecord) {
+                agg.visitContainedRecords(ptv);
-                continue;
+            } else {
+                ptv.visitRecord((Record) record);
             }
 
-            // Once the rows have been found in the list of records, start
-            //  writing out the blocked row information. This includes the DBCell references
-            pos += record.serialize(pos, data);
-
             // If the BOF record was just serialized then add the IndexRecord
             if (record instanceof BOFRecord) {
               if (!haveSerializedIndex) {
@@ -649,50 +587,42 @@ public final class Sheet implements Model {
                 // If there are diagrams, they have their own BOFRecords,
                 //  and one shouldn't go in after that!
                 if (_isUncalced) {
-                    UncalcedRecord rec = new UncalcedRecord();
+                    ptv.visitRecord(new UncalcedRecord());
-                    pos += rec.serialize(pos, data);
                 }
                 //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) {
-                  pos += serializeIndexRecord(k, pos, data);
+                    // find forward distance to first RowRecord
+                    int initRecsSize = getSizeOfInitialSheetRecords(k);
+                    int currentPos = ptv.getPosition();
+                    ptv.visitRecord(_rowsAggregate.createIndexRecord(currentPos, initRecsSize));
                 }
               }
             }
         }
-        if (log.check( POILogger.DEBUG )) {
-            log.log(POILogger.DEBUG, "Sheet.serialize returning ");
-        }
-        return pos-offset;
     }
-
     /**
-     * @param indexRecordOffset also happens to be the end of the BOF record
+     * 'initial sheet records' are between INDEX and the 'Row Blocks'
-     * @return the size of the serialized INDEX record
+     * @param bofRecordIndex index of record after which INDEX record is to be placed
+     * @return count of bytes from end of INDEX record to first ROW record.
      */
-    private int serializeIndexRecord(int bofRecordIndex, int indexRecordOffset, byte[] data) {
+    private int getSizeOfInitialSheetRecords(int bofRecordIndex) {
 
-        // 'initial sheet records' are between INDEX and first ROW record.
+        int result = 0;
-        int sizeOfInitialSheetRecords = 0;
         // start just after BOF record (INDEX is not present in this list)
         for (int j = bofRecordIndex + 1; j < records.size(); j++) {
-            RecordBase tmpRec = ((RecordBase) records.get(j));
+            RecordBase tmpRec = (RecordBase) records.get(j);
-            if (tmpRec instanceof UncalcedRecord) {
-                continue;
-            }
             if (tmpRec instanceof RowRecordsAggregate) {
                 break;
             }
-            sizeOfInitialSheetRecords += tmpRec.getRecordSize();
+            result += tmpRec.getRecordSize();
         }
         if (_isUncalced) {
-            sizeOfInitialSheetRecords += UncalcedRecord.getStaticRecordSize();
+            result += UncalcedRecord.getStaticRecordSize();
         }
-        IndexRecord index = _rowsAggregate.createIndexRecord(indexRecordOffset, sizeOfInitialSheetRecords);
+        return result;
-        return index.serialize(indexRecordOffset, data);
     }
 
-
     /**
      * Create a row record.  (does not add it to the records contained in this sheet)
      */
@@ -1351,32 +1281,6 @@ public final class Sheet implements Model {
         }
     }
 
-    /**
-     * @return the serialized size of this sheet
-     */
-    public int getSize() {
-        int retval = 0;
-
-        for ( int k = 0; k < records.size(); k++) {
-            RecordBase record = (RecordBase) records.get(k);
-            if (record instanceof UncalcedRecord) {
-                // skip the UncalcedRecord if present, it's only encoded if the isUncalced flag is set
-                continue;
-            }
-            retval += record.getRecordSize();
-        }
-        // add space for IndexRecord if needed
-        if (_rowsAggregate != null) {
-            // rowsAggregate knows how to make the index record
-            retval += IndexRecord.getRecordSizeForBlockCount(_rowsAggregate.getRowBlockCount());
-        }
-        // Add space for UncalcedRecord
-        if (_isUncalced) {
-            retval += UncalcedRecord.getStaticRecordSize();
-        }
-        return retval;
-    }
-
     public List getRecords()
     {
         return records;
@@ -1937,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,13 +28,11 @@ 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;
@@ -43,6 +40,10 @@ public final class ArrayRecord extends Record {
 
 	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/RecordAggregate.java

@@ -36,10 +36,16 @@ public abstract class RecordAggregate extends RecordBase {
 	protected final void fillFields(RecordInputStream in) {
 		throw new RuntimeException("Should not be called");
 	}
-    public final short getSid() {
+	public final short getSid() {
 		throw new RuntimeException("Should not be called");
-    }
+	}
 
+	/**
+	 * Visit each of the atomic BIFF records contained in this {@link RecordAggregate} in the order
+	 * that they should be written to file.  Implementors may or may not return the actual 
+	 * {@link Record}s being used to manage POI's internal implementation.  Callers should not
+	 * assume either way, and therefore only attempt to modify those {@link Record}s after cloning
+	 */
 	public abstract void visitContainedRecords(RecordVisitor rv);
 	
 	public final int serialize(int offset, byte[] data) {
@@ -94,4 +100,27 @@ public abstract class RecordAggregate extends RecordBase {
 			_totalSize += r.getRecordSize();
 		}
 	}
+	/**
+	 * A wrapper for {@link RecordVisitor} which accumulates the sizes of all
+	 * records visited.
+	 */
+	public static final class PositionTrackingVisitor implements RecordVisitor {
+		private final RecordVisitor _rv;
+		private int _position;
+
+		public PositionTrackingVisitor(RecordVisitor rv, int initialPosition) {
+			_rv = rv;
+			_position = initialPosition;
+		}
+		public void visitRecord(Record r) {
+			_position += r.getRecordSize();
+			_rv.visitRecord(r);
+		}
+		public void setPosition(int position) {
+			_position = position;
+		}
+		public int getPosition() {
+			return _position;
+		}
+	}
 }

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
@@ -95,7 +97,7 @@ public final class RowRecordsAggregate extends RecordAggregate {
         // 0x01C2 // several
         // 0x0034 // few
         // No documentation could be found for these
-        
+
         // keep the unknown records for re-serialization
         _unknownRecords.add(rec);
     }
@@ -147,7 +149,7 @@ public final class RowRecordsAggregate extends RecordAggregate {
     {
         return _lastrow;
     }
-    
+
     /** Returns the number of row blocks.
      * <p/>The row blocks are goupings of rows that contain the DBCell record
      * after them
@@ -209,7 +211,7 @@ public final class RowRecordsAggregate extends RecordAggregate {
       }
       return row.getRowNumber();
     }
-    
+
     private int visitRowRecordsForBlock(int blockIndex, RecordVisitor rv) {
         final int startIndex = blockIndex*DBCellRecord.BLOCK_SIZE;
         final int endIndex = startIndex + DBCellRecord.BLOCK_SIZE;
@@ -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.
@@ -273,8 +277,7 @@ public final class RowRecordsAggregate extends RecordAggregate {
     public Iterator getIterator() {
         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

@@ -292,23 +292,20 @@ public class HSSFCell implements Cell {
         {
 
             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/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java

@@ -51,6 +51,7 @@ import org.apache.poi.hssf.record.RecordFactory;
 import org.apache.poi.hssf.record.SSTRecord;
 import org.apache.poi.hssf.record.UnicodeString;
 import org.apache.poi.hssf.record.UnknownRecord;
+import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
 import org.apache.poi.hssf.record.formula.Area3DPtg;
 import org.apache.poi.hssf.record.formula.MemFuncPtg;
 import org.apache.poi.hssf.record.formula.NameXPtg;
@@ -109,7 +110,7 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
      */
 
     private ArrayList names;
-    
+
     /**
      * this holds the HSSFFont objects attached to this workbook.
      * We only create these from the low level records as required.
@@ -129,7 +130,7 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
      * someplace else.
      */
     private HSSFDataFormat formatter;
-    
+
     /**
      * The policy to apply in the event of missing or
      *  blank cells when fetching from a row.
@@ -380,7 +381,7 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
     /**
      * Sets the policy on what to do when
      *  getting missing or blank cells from a row.
-     * This will then apply to all calls to 
+     * This will then apply to all calls to
      *  {@link HSSFRow.getCell()}. See
      *  {@link MissingCellPolicy}
      */
@@ -403,17 +404,17 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
     private void validateSheetIndex(int index) {
         int lastSheetIx = _sheets.size() - 1;
         if (index < 0 || index > lastSheetIx) {
-            throw new IllegalArgumentException("Sheet index (" 
+            throw new IllegalArgumentException("Sheet index ("
                     + index +") is out of range (0.." +    lastSheetIx + ")");
         }
     }
-    
+
     /**
      * Selects a single sheet. This may be different to
-     * the 'active' sheet (which is the sheet with focus).  
+     * the 'active' sheet (which is the sheet with focus).
      */
     public void setSelectedTab(int index) {
-        
+
         validateSheetIndex(index);
         int nSheets = _sheets.size();
         for (int i=0; i<nSheets; i++) {
@@ -429,7 +430,7 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
         setSelectedTab((int)index);
     }
     public void setSelectedTabs(int[] indexes) {
-        
+
         for (int i = 0; i < indexes.length; i++) {
             validateSheetIndex(indexes[i]);
         }
@@ -441,7 +442,7 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
                     bSelect = true;
                     break;
                 }
-                
+
             }
                getSheetAt(i).setSelected(bSelect);
         }
@@ -453,7 +454,7 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
      * 'Selected' sheet(s) is a distinct concept.
      */
     public void setActiveSheet(int index) {
-        
+
         validateSheetIndex(index);
         int nSheets = _sheets.size();
         for (int i=0; i<nSheets; i++) {
@@ -474,13 +475,13 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
     }
     /**
      * deprecated May 2008
-     * @deprecated - Misleading name - use getActiveSheetIndex() 
+     * @deprecated - Misleading name - use getActiveSheetIndex()
      */
     public short getSelectedTab() {
         return (short) getActiveSheetIndex();
     }
 
-    
+
     /**
      * sets the first tab that is displayed in the list of tabs
      * in excel.
@@ -491,7 +492,7 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
     }
     /**
      * deprecated May 2008
-     * @deprecated - Misleading name - use setFirstVisibleTab() 
+     * @deprecated - Misleading name - use setFirstVisibleTab()
      */
     public void setDisplayedTab(short index) {
        setFirstVisibleTab(index);
@@ -505,7 +506,7 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
     }
     /**
      * deprecated May 2008
-     * @deprecated - Misleading name - use getFirstVisibleTab() 
+     * @deprecated - Misleading name - use getFirstVisibleTab()
      */
     public short getDisplayedTab() {
         return (short) getFirstVisibleTab();
@@ -698,7 +699,7 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
     /**
      * create an HSSFSheet for this HSSFWorkbook, adds it to the sheets and
      * returns the high level representation. Use this to create new sheets.
-     * 
+     *
      * @param sheetname
      *            sheetname to set for the sheet.
      * @return HSSFSheet representing the new sheet.
@@ -781,16 +782,16 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
 
     /**
      * Removes sheet at the given index.<p/>
-     * 
+     *
-     * Care must be taken if the removed sheet is the currently active or only selected sheet in 
+     * Care must be taken if the removed sheet is the currently active or only selected sheet in
-     * the workbook. There are a few situations when Excel must have a selection and/or active 
+     * the workbook. There are a few situations when Excel must have a selection and/or active
      * sheet. (For example when printing - see Bug 40414).<br/>
-     * 
+     *
      * This method makes sure that if the removed sheet was active, another sheet will become
      * active in its place.  Furthermore, if the removed sheet was the only selected sheet, another
-     * sheet will become selected.  The newly active/selected sheet will have the same index, or 
+     * sheet will become selected.  The newly active/selected sheet will have the same index, or
      * one less if the removed sheet was the last in the workbook.
-     * 
+     *
      * @param index of the sheet  (0-based)
      */
     public void removeSheetAt(int index) {
@@ -1023,7 +1024,7 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
         if(fontindex == Short.MAX_VALUE){
             throw new IllegalArgumentException("Maximum number of fonts was exceeded");
         }
-        
+
         // Ask getFontAt() to build it for us,
         //  so it gets properly cached
         return getFontAt(fontindex);
@@ -1039,7 +1040,7 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
         for (short i=0; i<=getNumberOfFonts(); i++) {
             // Remember - there is no 4!
             if(i == 4) continue;
-            
+
             HSSFFont hssfFont = getFontAt(i);
             if (hssfFont.getBoldweight() == boldWeight
                     && hssfFont.getColor() == color
@@ -1089,7 +1090,7 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
 
         return retval;
     }
-    
+
     /**
      * Reset the fonts cache, causing all new calls
      *  to getFontAt() to create new objects.
@@ -1179,6 +1180,37 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
         //poifs.writeFilesystem(stream);
     }
 
+    /**
+     * Totals the sizes of all sheet records and eventually serializes them
+     */
+    private static final class SheetRecordCollector implements RecordVisitor {
+
+        private List _list;
+        private int _totalSize;
+
+        public SheetRecordCollector() {
+            _totalSize = 0;
+            _list = new ArrayList(128);
+        }
+        public int getTotalSize() {
+            return _totalSize;
+        }
+        public void visitRecord(Record r) {
+            _list.add(r);
+            _totalSize+=r.getRecordSize();
+        }
+        public int serialize(int offset, byte[] data) {
+            int result = 0;
+            int nRecs = _list.size();
+            for(int i=0; i<nRecs; i++) {
+                Record rec = (Record)_list.get(i);
+                result += rec.serialize(offset + result, data);
+            }
+            return result;
+        }
+    }
+
+
     /**
      * Method getBytes - get the bytes of just the HSSF portions of the XLS file.
      * Use this to construct a POI POIFSFileSystem yourself.
@@ -1190,13 +1222,11 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
      * @see org.apache.poi.hssf.model.Workbook
      * @see org.apache.poi.hssf.model.Sheet
      */
-
+    public byte[] getBytes() {
-    public byte[] getBytes()
-    {
         if (log.check( POILogger.DEBUG )) {
             log.log(DEBUG, "HSSFWorkbook.getBytes()");
         }
-        
+
         HSSFSheet[] sheets = getSheets();
         int nSheets = sheets.length;
 
@@ -1209,26 +1239,27 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
         int totalsize = workbook.getSize();
 
         // pre-calculate all the sheet sizes and set BOF indexes
-        int[] estimatedSheetSizes = new int[nSheets];
+        SheetRecordCollector[] srCollectors = new SheetRecordCollector[nSheets];
         for (int k = 0; k < nSheets; k++) {
             workbook.setSheetBof(k, totalsize);
-            int sheetSize = sheets[k].getSheet().getSize();
+            SheetRecordCollector src = new SheetRecordCollector();
-            estimatedSheetSizes[k] = sheetSize;
+            sheets[k].getSheet().visitContainedRecords(src, totalsize);
-            totalsize += sheetSize;
+            totalsize += src.getTotalSize();
+            srCollectors[k] = src;
         }
 
-
         byte[] retval = new byte[totalsize];
         int pos = workbook.serialize(0, retval);
 
         for (int k = 0; k < nSheets; k++) {
-            int serializedSize = sheets[k].getSheet().serialize(pos, retval);
+            SheetRecordCollector src = srCollectors[k];
-            if (serializedSize != estimatedSheetSizes[k]) {
+            int serializedSize = src.serialize(pos, retval);
+            if (serializedSize != src.getTotalSize()) {
                 // Wrong offset values have been passed in the call to setSheetBof() above.
-                // For books with more than one sheet, this discrepancy would cause excel 
+                // For books with more than one sheet, this discrepancy would cause excel
                 // to report errors and loose data while reading the workbook
-                throw new IllegalStateException("Actual serialized sheet size (" + serializedSize 
+                throw new IllegalStateException("Actual serialized sheet size (" + serializedSize
-                        + ") differs from pre-calculated size (" + estimatedSheetSizes[k] 
+                        + ") differs from pre-calculated size (" + src.getTotalSize()
                         + ") for sheet (" + k + ")");
                 // TODO - add similar sanity check to ensure that Sheet.serializeIndexRecord() does not write mis-aligned offsets either
             }
@@ -1671,11 +1702,11 @@ public class HSSFWorkbook extends POIDocument implements org.apache.poi.ss.userm
     }
 
     /**
-     * Note - This method should only used by POI internally.  
+     * Note - This method should only used by POI internally.
      * It may get deleted or change definition in future POI versions
      */
-	public NameXPtg getNameXPtg(String name) {
+    public NameXPtg getNameXPtg(String name) {
-		return workbook.getNameXPtg(name);		
+        return workbook.getNameXPtg(name);
-	}
+    }
 
 }

src/java/org/apache/poi/util/StringUtil.java

@@ -68,7 +68,7 @@ public class StringUtil {
 			throw new ArrayIndexOutOfBoundsException("Illegal offset");
 		}
 		if ((len < 0) || (((string.length - offset) / 2) < len)) {
-			throw new IllegalArgumentException("Illegal length");
+			throw new IllegalArgumentException("Illegal length " + len);
 		}
 
 		try {

src/scratchpad/src/org/apache/poi/hpbf/dev/PLCDumper.java

@@ -20,17 +20,11 @@ import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 
-import org.apache.poi.ddf.DefaultEscherRecordFactory;
-import org.apache.poi.ddf.EscherRecord;
 import org.apache.poi.hpbf.HPBFDocument;
 import org.apache.poi.hpbf.model.QuillContents;
 import org.apache.poi.hpbf.model.qcbits.QCBit;
-import org.apache.poi.poifs.filesystem.DirectoryNode;
-import org.apache.poi.poifs.filesystem.DocumentEntry;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.poi.util.HexDump;
-import org.apache.poi.util.LittleEndian;
-import org.apache.poi.util.StringUtil;
 
 /**
  * For dumping out the PLC contents of QC Bits of a
@@ -41,8 +35,8 @@ public class PLCDumper {
 	private HPBFDocument doc;
 	private QuillContents qc;
 	
-	public PLCDumper(HPBFDocument doc) {
+	public PLCDumper(HPBFDocument hpbfDoc) {
-		this.doc = doc;
+		doc = hpbfDoc;
 		qc = doc.getQuillContents();
 	}
 	public PLCDumper(POIFSFileSystem fs) throws IOException {
@@ -67,7 +61,6 @@ public class PLCDumper {
 	}
 	
 	private void dumpPLC() {	
-		QuillContents qc = doc.getQuillContents();
 		QCBit[] bits = qc.getBits();
 		
 		for(int i=0; i<bits.length; i++) {
@@ -82,8 +75,8 @@ public class PLCDumper {
 		System.out.println("");
 		System.out.println("Dumping " + bit.getBitType() + " bit at " + index);
 		System.out.println("  Is a " + bit.getThingType() + ", number is " + bit.getOptA());
-		System.out.println("  Starts at " + bit.getDataOffset() + " (" + Integer.toHexString(bit.getDataOffset()) + ")");
+		System.out.println("  Starts at " + bit.getDataOffset() + " (0x" + Integer.toHexString(bit.getDataOffset()) + ")");
-		System.out.println("  Runs for  " + bit.getLength() + " (" + Integer.toHexString(bit.getLength()) + ")");
+		System.out.println("  Runs for  " + bit.getLength() + " (0x" + Integer.toHexString(bit.getLength()) + ")");
 		
 		System.out.println(HexDump.dump(bit.getData(), 0, 0));
 	}

src/scratchpad/src/org/apache/poi/hpbf/extractor/PublisherTextExtractor.java

@@ -24,6 +24,7 @@ import org.apache.poi.POIOLE2TextExtractor;
 import org.apache.poi.hpbf.HPBFDocument;
 import org.apache.poi.hpbf.model.qcbits.QCBit;
 import org.apache.poi.hpbf.model.qcbits.QCTextBit;
+import org.apache.poi.hpbf.model.qcbits.QCPLCBit.Type12;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 
 /**
@@ -31,6 +32,7 @@ import org.apache.poi.poifs.filesystem.POIFSFileSystem;
  */
 public class PublisherTextExtractor extends POIOLE2TextExtractor {
 	private HPBFDocument doc;
+	private boolean hyperlinksByDefault = false;
 	
 	public PublisherTextExtractor(HPBFDocument doc) {
 		super(doc);
@@ -43,6 +45,16 @@ public class PublisherTextExtractor extends POIOLE2TextExtractor {
 		this(new POIFSFileSystem(is));
 	}
 	
+	/**
+	 * Should a call to getText() return hyperlinks inline
+	 *  with the text?
+	 * Default is no
+	 */
+	public void setHyperlinksByDefault(boolean hyperlinksByDefault) {
+		this.hyperlinksByDefault = hyperlinksByDefault;
+	}
+
+	
 	public String getText() {
 		StringBuffer text = new StringBuffer();
 		
@@ -55,6 +67,24 @@ public class PublisherTextExtractor extends POIOLE2TextExtractor {
 			}
 		}
 		
+		// If requested, add in the hyperlinks
+		// Ideally, we'd do these inline, but the hyperlink
+		//  positions are relative to the text area the
+		//  hyperlink is in, and we have yet to figure out
+		//  how to tie that together.
+		if(hyperlinksByDefault) {
+			for(int i=0; i<bits.length; i++) {
+				if(bits[i] != null && bits[i] instanceof Type12) {
+					Type12 hyperlinks = (Type12)bits[i];
+					for(int j=0; j<hyperlinks.getNumberOfHyperlinks(); j++) {
+						text.append("<");
+						text.append(hyperlinks.getHyperlink(j));
+						text.append(">\n");
+					}
+				}
+			}
+		}
+		
 		// Get more text
 		// TODO
 		

src/scratchpad/src/org/apache/poi/hpbf/model/QuillContents.java

@@ -19,6 +19,7 @@ package org.apache.poi.hpbf.model;
 import java.io.IOException;
 
 import org.apache.poi.hpbf.model.qcbits.QCBit;
+import org.apache.poi.hpbf.model.qcbits.QCPLCBit;
 import org.apache.poi.hpbf.model.qcbits.QCTextBit;
 import org.apache.poi.hpbf.model.qcbits.UnknownQCBit;
 import org.apache.poi.poifs.filesystem.DirectoryNode;
@@ -64,6 +65,8 @@ public final class QuillContents extends HPBFPart {
 				// Create
 				if(bitType.equals("TEXT")) {
 					bits[i] = new QCTextBit(thingType, bitType, bitData);
+				} else if(bitType.equals("PLC ")) {
+					bits[i] = QCPLCBit.createQCPLCBit(thingType, bitType, bitData);
 				} else {
 					bits[i] = new UnknownQCBit(thingType, bitType, bitData);
 				}

src/scratchpad/src/org/apache/poi/hpbf/model/qcbits/QCPLCBit.java

@@ -0,0 +1,274 @@
+/* ====================================================================
+   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.hpbf.model.qcbits;
+
+import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.StringUtil;
+
+
+/**
+ * A "PLC " (PLC) based bit of Quill Contents. The exact
+ *  format is determined by the type of the PLCs.
+ */
+public class QCPLCBit extends QCBit {
+	protected int numberOfPLCs;
+	protected int typeOfPLCS;
+	/** 
+	 * The data which goes before the main PLC entries.
+	 * This is apparently always made up of 2 byte 
+	 *  un-signed ints..
+	 */
+	protected int[] preData;
+	/** The first value of each PLC, normally 4 bytes */
+	protected long[] plcValA;
+	/** The second value of each PLC, normally 4 bytes */
+	protected long[] plcValB;
+	
+	
+	private QCPLCBit(String thingType, String bitType, byte[] data) {
+		super(thingType, bitType, data);
+		
+		// First four bytes are the number
+		numberOfPLCs = (int)LittleEndian.getUInt(data, 0);
+		
+		// Next four bytes are the type
+		typeOfPLCS = (int)LittleEndian.getUInt(data, 4);
+		
+		// Init the arrays that we can
+		plcValA = new long[numberOfPLCs];
+		plcValB = new long[numberOfPLCs];
+	}
+	
+	
+	
+	public int getNumberOfPLCs() {
+		return numberOfPLCs;
+	}
+	public int getTypeOfPLCS() {
+		return typeOfPLCS;
+	}
+
+	public int[] getPreData() {
+		return preData;
+	}
+
+	public long[] getPlcValA() {
+		return plcValA;
+	}
+	public long[] getPlcValB() {
+		return plcValB;
+	}
+
+
+
+	public static QCPLCBit createQCPLCBit(String thingType, String bitType, byte[] data) {
+		// Grab the type
+		int type = (int)LittleEndian.getUInt(data, 4);
+		switch(type) {
+			case 0:
+				return new Type0(thingType, bitType, data);
+			case 4:
+				return new Type4(thingType, bitType, data);
+			case 8:
+				return new Type8(thingType, bitType, data);
+			case 12: // 0xc
+				return new Type12(thingType, bitType, data);
+			default:
+				throw new IllegalArgumentException("Sorry, I don't know how to deal with PLCs of type " + type);
+		}
+	}
+
+	
+	/**
+	 * Type 0 seem to be somewhat rare. They have 8 bytes of pre-data,
+	 *  then 2x 2 byte values.
+	 */
+	public static class Type0 extends QCPLCBit {
+		private Type0(String thingType, String bitType, byte[] data) {
+			super(thingType, bitType, data);
+			
+			// Grab our 4x pre-data
+			preData = new int[4];
+			preData[0] = LittleEndian.getUShort(data, 8+0);
+			preData[1] = LittleEndian.getUShort(data, 8+2);
+			preData[2] = LittleEndian.getUShort(data, 8+4);
+			preData[3] = LittleEndian.getUShort(data, 8+6);
+			
+			// And grab the 2 byte values
+			for(int i=0; i<numberOfPLCs; i++) {
+				plcValA[i] = LittleEndian.getUShort(data, 16+(4*i));
+				plcValB[i] = LittleEndian.getUShort(data, 16+(4*i)+2);
+			}
+		}
+	}
+	
+	/**
+	 * Type 4 is quite common. They have 8 bytes of pre-data,
+	 *  then 2x 4 byte values.
+	 */
+	public static class Type4 extends QCPLCBit {
+		private Type4(String thingType, String bitType, byte[] data) {
+			super(thingType, bitType, data);
+			
+			// Grab our 4x pre-data
+			preData = new int[4];
+			preData[0] = LittleEndian.getUShort(data, 8+0);
+			preData[1] = LittleEndian.getUShort(data, 8+2);
+			preData[2] = LittleEndian.getUShort(data, 8+4);
+			preData[3] = LittleEndian.getUShort(data, 8+6);
+			
+			// And grab the 4 byte values
+			for(int i=0; i<numberOfPLCs; i++) {
+				plcValA[i] = LittleEndian.getUInt(data, 16+(8*i));
+				plcValB[i] = LittleEndian.getUInt(data, 16+(8*i)+4);
+			}
+		}
+	}
+	
+	/**
+	 * Type 8 is quite common. They have 14 bytes of pre-data,
+	 *  then 2x 4 byte values.
+	 */
+	public static class Type8 extends QCPLCBit {
+		private Type8(String thingType, String bitType, byte[] data) {
+			super(thingType, bitType, data);
+			
+			// Grab our 7x pre-data
+			preData = new int[7];
+			preData[0] = LittleEndian.getUShort(data, 8+0);
+			preData[1] = LittleEndian.getUShort(data, 8+2);
+			preData[2] = LittleEndian.getUShort(data, 8+4);
+			preData[3] = LittleEndian.getUShort(data, 8+6);
+			preData[4] = LittleEndian.getUShort(data, 8+8);
+			preData[5] = LittleEndian.getUShort(data, 8+10);
+			preData[6] = LittleEndian.getUShort(data, 8+12);
+			
+			// And grab the 4 byte values
+			for(int i=0; i<numberOfPLCs; i++) {
+				plcValA[i] = LittleEndian.getUInt(data, 22+(8*i));
+				plcValB[i] = LittleEndian.getUInt(data, 22+(8*i)+4);
+			}
+		}
+	}
+	
+	/**
+	 * Type 12 holds hyperlinks, and is very complex.
+	 * There is normally one of these for each text
+	 *  area that contains at least one hyperlinks.
+	 * The character offsets are relative to the start
+	 *  of the text area that this applies to.
+	 */
+	public static class Type12 extends QCPLCBit {
+		private String[] hyperlinks;
+		
+		private static final int oneStartsAt = 0x4c;
+		private static final int twoStartsAt = 0x68;
+		private static final int threePlusIncrement = 22;
+		
+		private Type12(String thingType, String bitType, byte[] data) {
+			super(thingType, bitType, data);
+			
+			// How many hyperlinks do we really have?
+			// (zero hyperlinks gets numberOfPLCs=1)
+			if(data.length == 0x34) {
+				hyperlinks = new String[0];
+			} else {
+				hyperlinks = new String[numberOfPLCs];
+			}
+			
+			// We have 4 bytes, then the start point of each
+			//  hyperlink, then the end point of the text.
+			preData = new int[1+numberOfPLCs+1];
+			for(int i=0; i<preData.length; i++) {
+				preData[i] = (int)LittleEndian.getUInt(data, 8+(i*4));
+			}
+			
+			// Then we have a whole bunch of stuff, which grows
+			//  with the number of hyperlinks
+			// For now, we think these are shorts
+			int at = 8+4+(numberOfPLCs*4)+4;
+			int until = 0x34;
+			if(numberOfPLCs == 1 && hyperlinks.length == 1) {
+				until = oneStartsAt;
+			} else if(numberOfPLCs >= 2) {
+				until = twoStartsAt + (numberOfPLCs-2)*threePlusIncrement;
+			}
+			
+			plcValA = new long[(until-at)/2];
+			plcValB = new long[0];
+			for(int i=0; i<plcValA.length; i++) {
+				plcValA[i] = LittleEndian.getUShort(data, at+(i*2));
+			}
+			
+			// Finally, we have a series of lengths + hyperlinks
+			at = until;
+			for(int i=0; i<hyperlinks.length; i++) {
+				int len = LittleEndian.getUShort(data, at);
+				int first = LittleEndian.getUShort(data, at+2);
+				if(first == 0) {
+					// Crazy special case
+					// Length is in bytes, from the start
+					// Hyperlink appears to be empty
+					hyperlinks[i] = "";
+					at += len;
+				} else {
+					// Normal case. Length is in characters
+					hyperlinks[i] = StringUtil.getFromUnicodeLE(data, at+2, len);
+					at += 2 + (2*len);
+				}
+			}
+		}
+		
+		/**
+		 * Returns the number of hyperlinks, which should 
+		 *  either be zero, or the number of PLC bits
+		 */
+		public int getNumberOfHyperlinks() {
+			return hyperlinks.length;
+		}
+
+		/**
+		 * Returns the URL of the hyperlink at the
+		 *  given index.
+		 * @param number The hyperlink number, zero based
+		 */
+		public String getHyperlink(int number) {
+			return hyperlinks[number];
+		}
+		/**
+		 * Returns where in the text (in characters) the
+		 *  hyperlink at the given index starts 
+		 *  applying to.
+		 * This position is relative to the text area that this
+		 *  PLCBit applies to.
+		 * @param number The hyperlink number, zero based
+		 */
+		public int getTextStartAt(int number) {
+			return preData[1+number];
+		}
+		/**
+		 * Returns where in the text that this block
+		 *  of hyperlinks stops applying to. Normally,
+		 *  but not always the end of the text.
+		 * This position is relative to the text area that this
+		 *  PLCBit applies to.
+		 */
+		public int getAllTextEndAt() {
+			return preData[numberOfPLCs+1];
+		}
+	}
+}

src/scratchpad/testcases/org/apache/poi/hpbf/extractor/TextPublisherTextExtractor.java

@@ -134,4 +134,41 @@ public class TextPublisherTextExtractor extends TestCase {
 		assertEquals(s2007, s2000);
 		assertEquals(s2007, s98);
 	}
+	
+	/**
+	 * Test that the hyperlink extraction stuff works as well
+	 *  as we can hope it to.
+	 */
+	public void testWithHyperlinks() throws Exception {
+		File f = new File(dir, "LinkAt10.pub");
+		HPBFDocument doc = new HPBFDocument(
+				new FileInputStream(f)
+		);
+
+		PublisherTextExtractor ext = 
+			new PublisherTextExtractor(doc);
+		ext.getText();
+		
+		// Default is no hyperlinks
+		assertEquals("1234567890LINK\n", ext.getText());
+		
+		// Turn on
+		ext.setHyperlinksByDefault(true);
+		assertEquals("1234567890LINK\n<http://poi.apache.org/>\n", ext.getText());
+		
+		
+		// Now a much more complex document
+		f = new File(dir, "Sample.pub");
+		ext = new PublisherTextExtractor(new FileInputStream(f));
+		ext.setHyperlinksByDefault(true);
+		String text = ext.getText();
+		
+		assertTrue(text.endsWith(
+				"<http://poi.apache.org/>\n" +
+				"<C:\\Documents and Settings\\Nick\\My Documents\\Booleans.xlsx>\n" +
+				"<>\n" +
+				"<mailto:dev@poi.apache.org?subject=HPBF>\n" +
+				"<mailto:dev@poi.apache.org?subject=HPBF>\n"
+		));
+	}
 }

src/scratchpad/testcases/org/apache/poi/hpbf/model/TestQuillContents.java

@@ -21,6 +21,10 @@ import java.io.FileInputStream;
 
 import org.apache.poi.hpbf.HPBFDocument;
 import org.apache.poi.hpbf.model.qcbits.QCTextBit;
+import org.apache.poi.hpbf.model.qcbits.QCPLCBit.Type12;
+import org.apache.poi.hpbf.model.qcbits.QCPLCBit.Type0;
+import org.apache.poi.hpbf.model.qcbits.QCPLCBit.Type4;
+import org.apache.poi.hpbf.model.qcbits.QCPLCBit.Type8;
 
 import junit.framework.TestCase;
 
@@ -77,4 +81,354 @@ public class TestQuillContents extends TestCase {
 		assertTrue(t.startsWith("This is some text on the first page"));
 		assertTrue(t.endsWith("Within doc to page 1\r"));
 	}
+	
+	public void testPLC() throws Exception {
+		File f = new File(dir, "Simple.pub");
+		HPBFDocument doc = new HPBFDocument(
+				new FileInputStream(f)
+		);
+		
+		QuillContents qc = doc.getQuillContents();
+		assertEquals(20, qc.getBits().length);
+		
+		assertTrue(qc.getBits()[9] instanceof Type4);
+		assertTrue(qc.getBits()[10] instanceof Type4);
+		assertTrue(qc.getBits()[12] instanceof Type8);
+		
+		Type4 plc9 = (Type4)qc.getBits()[9];
+		Type4 plc10 = (Type4)qc.getBits()[10];
+		Type8 plc12 = (Type8)qc.getBits()[12];
+		
+		
+		assertEquals(1, plc9.getNumberOfPLCs());
+		assertEquals(4, plc9.getPreData().length);
+		assertEquals(1, plc9.getPlcValA().length);
+		assertEquals(1, plc9.getPlcValB().length);
+		
+		assertEquals(0, plc9.getPreData()[0]);
+		assertEquals(0, plc9.getPreData()[1]);
+		assertEquals(0, plc9.getPreData()[2]);
+		assertEquals(0, plc9.getPreData()[3]);
+		assertEquals(0x356, plc9.getPlcValA()[0]);
+		assertEquals(0x600, plc9.getPlcValB()[0]);
+		
+		
+		assertEquals(1, plc10.getNumberOfPLCs());
+		assertEquals(4, plc10.getPreData().length);
+		assertEquals(1, plc10.getPlcValA().length);
+		assertEquals(1, plc10.getPlcValB().length);
+		
+		assertEquals(0, plc10.getPreData()[0]);
+		assertEquals(0, plc10.getPreData()[1]);
+		assertEquals(0, plc10.getPreData()[2]);
+		assertEquals(0, plc10.getPreData()[3]);
+		assertEquals(0x356, plc10.getPlcValA()[0]);
+		assertEquals(0x800, plc10.getPlcValB()[0]);
+		
+		assertEquals(2, plc12.getNumberOfPLCs());
+		assertEquals(7, plc12.getPreData().length);
+		assertEquals(2, plc12.getPlcValA().length);
+		assertEquals(2, plc12.getPlcValB().length);
+		
+		assertEquals(0xff, plc12.getPreData()[0]);
+		assertEquals(0, plc12.getPreData()[1]);
+		assertEquals(0x3d, plc12.getPreData()[2]);
+		assertEquals(0, plc12.getPreData()[3]);
+		assertEquals(0x6e, plc12.getPreData()[4]);
+		assertEquals(0, plc12.getPreData()[5]);
+		assertEquals(0, plc12.getPreData()[6]);
+		assertEquals(0xa0000, plc12.getPlcValA()[0]);
+		assertEquals(0x22000000, plc12.getPlcValB()[0]);
+		assertEquals(0x05, plc12.getPlcValA()[1]);
+		assertEquals(0x04, plc12.getPlcValB()[1]);
+	}
+	
+	public void testComplexPLC() throws Exception {
+		File f = new File(dir, "Sample.pub");
+		HPBFDocument doc = new HPBFDocument(
+				new FileInputStream(f)
+		);
+		
+		QuillContents qc = doc.getQuillContents();
+		assertEquals(20, qc.getBits().length);
+		
+		assertTrue(qc.getBits()[10] instanceof Type4);
+		assertTrue(qc.getBits()[11] instanceof Type4);
+		assertTrue(qc.getBits()[13] instanceof Type0);
+		assertTrue(qc.getBits()[14] instanceof Type12);
+		assertTrue(qc.getBits()[15] instanceof Type12);
+		assertTrue(qc.getBits()[16] instanceof Type8);
+		
+		Type4 plc10 = (Type4)qc.getBits()[10];
+		Type4 plc11 = (Type4)qc.getBits()[11];
+		Type0 plc13 = (Type0)qc.getBits()[13];
+		Type12 plc14 = (Type12)qc.getBits()[14];
+		Type12 plc15 = (Type12)qc.getBits()[15];
+		Type8 plc16 = (Type8)qc.getBits()[16];
+		
+		
+		assertEquals(1, plc10.getNumberOfPLCs());
+		assertEquals(4, plc10.getPreData().length);
+		assertEquals(1, plc10.getPlcValA().length);
+		assertEquals(1, plc10.getPlcValB().length);
+		
+		assertEquals(0, plc10.getPreData()[0]);
+		assertEquals(0, plc10.getPreData()[1]);
+		assertEquals(0, plc10.getPreData()[2]);
+		assertEquals(0, plc10.getPreData()[3]);
+		assertEquals(0x5d0, plc10.getPlcValA()[0]);
+		assertEquals(0x800, plc10.getPlcValB()[0]);
+		
+		
+		assertEquals(2, plc11.getNumberOfPLCs());
+		assertEquals(4, plc11.getPreData().length);
+		assertEquals(2, plc11.getPlcValA().length);
+		assertEquals(2, plc11.getPlcValB().length);
+		
+		assertEquals(0, plc11.getPreData()[0]);
+		assertEquals(0, plc11.getPreData()[1]);
+		assertEquals(0, plc11.getPreData()[2]);
+		assertEquals(0, plc11.getPreData()[3]);
+		assertEquals(0x53a, plc11.getPlcValA()[0]);
+		assertEquals(0x5d0, plc11.getPlcValB()[0]);
+		assertEquals(0xa00, plc11.getPlcValA()[1]);
+		assertEquals(0xc00, plc11.getPlcValB()[1]);
+		
+		
+		assertEquals(5, plc13.getNumberOfPLCs());
+		assertEquals(4, plc13.getPreData().length);
+		assertEquals(5, plc13.getPlcValA().length);
+		assertEquals(5, plc13.getPlcValB().length);
+		
+		assertEquals(0xff00, plc13.getPreData()[0]);
+		assertEquals(0, plc13.getPreData()[1]);
+		assertEquals(0xf, plc13.getPreData()[2]);
+		assertEquals(0, plc13.getPreData()[3]);
+		assertEquals(0x19, plc13.getPlcValA()[0]);
+		assertEquals(0x00, plc13.getPlcValB()[0]);
+		assertEquals(0x27, plc13.getPlcValA()[1]);
+		assertEquals(0x00, plc13.getPlcValB()[1]);
+		assertEquals(0x36, plc13.getPlcValA()[2]);
+		assertEquals(0x00, plc13.getPlcValB()[2]);
+		assertEquals(0x42, plc13.getPlcValA()[3]);
+		assertEquals(0x00, plc13.getPlcValB()[3]);
+		assertEquals(0x50, plc13.getPlcValA()[4]);
+		assertEquals(0x00, plc13.getPlcValB()[4]);
+		
+		
+		// TODO - test the type 12s
+		
+		
+		assertEquals(6, plc16.getNumberOfPLCs());
+		assertEquals(7, plc16.getPreData().length);
+		assertEquals(6, plc16.getPlcValA().length);
+		assertEquals(6, plc16.getPlcValB().length);
+		
+		assertEquals(0xff, plc16.getPreData()[0]);
+		assertEquals(0, plc16.getPreData()[1]);
+		assertEquals(0x56, plc16.getPreData()[2]);
+		assertEquals(0, plc16.getPreData()[3]);
+		assertEquals(0x62, plc16.getPreData()[4]);
+		assertEquals(0, plc16.getPreData()[5]);
+		assertEquals(0x3e, plc16.getPreData()[6]);
+		assertEquals(0x500000, plc16.getPlcValA()[0]);
+		assertEquals(0x570000, plc16.getPlcValB()[0]);
+		assertEquals(0x4b0000, plc16.getPlcValA()[1]);
+		assertEquals(0x000000, plc16.getPlcValB()[1]);
+		assertEquals(0x0a0000, plc16.getPlcValA()[2]);
+		assertEquals(0x22000000, plc16.getPlcValB()[2]);
+		assertEquals(0x000005, plc16.getPlcValA()[3]);
+		assertEquals(0x000004, plc16.getPlcValB()[3]);
+		assertEquals(0x000004, plc16.getPlcValA()[4]);
+		assertEquals(0x000004, plc16.getPlcValB()[4]);
+		assertEquals(0x000004, plc16.getPlcValA()[5]);
+		assertEquals(0x000004, plc16.getPlcValB()[5]);
+	}
+	
+	public void testNoHyperlinks() throws Exception {
+		File f = new File(dir, "SampleNewsletter.pub");
+		HPBFDocument doc = new HPBFDocument(
+				new FileInputStream(f)
+		);
+		
+		QuillContents qc = doc.getQuillContents();
+		assertEquals(20, qc.getBits().length);
+		
+		Type12 plc18 = (Type12)qc.getBits()[18];
+		
+		assertEquals(1, plc18.getNumberOfPLCs());
+		assertEquals(0, plc18.getNumberOfHyperlinks());
+		assertEquals(0, plc18.getTextStartAt(0));
+		assertEquals(601, plc18.getAllTextEndAt());
+	}
+	
+	public void testSimpleHyperlink() throws Exception {
+		File f;
+		HPBFDocument doc;
+		QuillContents qc;
+		Type12 hlBit;
+		
+		// Link at 10
+		f = new File(dir, "LinkAt10.pub");
+		doc = new HPBFDocument(
+				new FileInputStream(f)
+		);
+		qc = doc.getQuillContents();
+		
+		hlBit = (Type12)qc.getBits()[12];
+		assertEquals(1, hlBit.getNumberOfPLCs());
+		assertEquals(1, hlBit.getNumberOfHyperlinks());
+		
+		assertEquals(10, hlBit.getTextStartAt(0));
+		assertEquals(15, hlBit.getAllTextEndAt());
+		assertEquals("http://poi.apache.org/", hlBit.getHyperlink(0));
+		
+		// Longer link at 10
+		f = new File(dir, "LinkAt10Longer.pub");
+		doc = new HPBFDocument(
+				new FileInputStream(f)
+		);
+		qc = doc.getQuillContents();
+		
+		hlBit = (Type12)qc.getBits()[12];
+		assertEquals(1, hlBit.getNumberOfPLCs());
+		assertEquals(1, hlBit.getNumberOfHyperlinks());
+		
+		assertEquals(10, hlBit.getTextStartAt(0));
+		assertEquals(15, hlBit.getAllTextEndAt());
+		assertEquals("http://poi.apache.org/hpbf/", hlBit.getHyperlink(0));
+		
+		// Link at 20
+		f = new File(dir, "LinkAt20.pub");
+		doc = new HPBFDocument(
+				new FileInputStream(f)
+		);
+		qc = doc.getQuillContents();
+		
+		hlBit = (Type12)qc.getBits()[12];
+		assertEquals(1, hlBit.getNumberOfPLCs());
+		assertEquals(1, hlBit.getNumberOfHyperlinks());
+		
+		assertEquals(20, hlBit.getTextStartAt(0));
+		assertEquals(25, hlBit.getAllTextEndAt());
+		assertEquals("http://poi.apache.org/", hlBit.getHyperlink(0));
+	}
+	
+	public void testManyHyperlinks() throws Exception {
+		File f;
+		HPBFDocument doc;
+		QuillContents qc;
+		Type12 hlBit;
+		
+		// Link at 10
+		f = new File(dir, "LinkAt10.pub");
+		doc = new HPBFDocument(
+				new FileInputStream(f)
+		);
+		qc = doc.getQuillContents();
+		
+		hlBit = (Type12)qc.getBits()[12];
+		assertEquals(1, hlBit.getNumberOfPLCs());
+		assertEquals(1, hlBit.getNumberOfHyperlinks());
+		
+		assertEquals(10, hlBit.getTextStartAt(0));
+		assertEquals(15, hlBit.getAllTextEndAt());
+		assertEquals("http://poi.apache.org/", hlBit.getHyperlink(0));
+		
+	}
+	
+	public void testHyperlinkDifferentVersions() throws Exception {
+		File f;
+		HPBFDocument doc;
+		QuillContents qc;
+		Type12 hlBitA;
+		Type12 hlBitB;
+		
+		// Latest version
+		f = new File(dir, "Sample.pub");
+		doc = new HPBFDocument(
+				new FileInputStream(f)
+		);
+		qc = doc.getQuillContents();
+		
+		hlBitA = (Type12)qc.getBits()[14];
+		assertEquals(2, hlBitA.getNumberOfPLCs());
+		assertEquals(2, hlBitA.getNumberOfHyperlinks());
+		
+		assertEquals(25, hlBitA.getTextStartAt(0));
+		assertEquals(72, hlBitA.getTextStartAt(1));
+		assertEquals(87, hlBitA.getAllTextEndAt());
+		assertEquals("http://poi.apache.org/", hlBitA.getHyperlink(0));
+		assertEquals("C:\\Documents and Settings\\Nick\\My Documents\\Booleans.xlsx", hlBitA.getHyperlink(1));
+		
+		hlBitB = (Type12)qc.getBits()[15];
+		assertEquals(3, hlBitB.getNumberOfPLCs());
+		assertEquals(3, hlBitB.getNumberOfHyperlinks());
+		
+		assertEquals(27, hlBitB.getTextStartAt(0));
+		assertEquals(37, hlBitB.getTextStartAt(1));
+		assertEquals(54, hlBitB.getTextStartAt(2));
+		assertEquals(75, hlBitB.getAllTextEndAt());
+		assertEquals("", hlBitB.getHyperlink(0));
+		assertEquals("mailto:dev@poi.apache.org?subject=HPBF", hlBitB.getHyperlink(1));
+		assertEquals("mailto:dev@poi.apache.org?subject=HPBF", hlBitB.getHyperlink(2));
+		
+		// 2000 version
+		f = new File(dir, "Sample2000.pub");
+		doc = new HPBFDocument(
+				new FileInputStream(f)
+		);
+		qc = doc.getQuillContents();
+		
+		hlBitA = (Type12)qc.getBits()[13];
+		assertEquals(2, hlBitA.getNumberOfPLCs());
+		assertEquals(2, hlBitA.getNumberOfHyperlinks());
+		
+		assertEquals(25, hlBitA.getTextStartAt(0));
+		assertEquals(72, hlBitA.getTextStartAt(1));
+		assertEquals(87, hlBitA.getAllTextEndAt());
+		assertEquals("http://poi.apache.org/", hlBitA.getHyperlink(0));
+		assertEquals("C:\\Documents and Settings\\Nick\\My Documents\\Booleans.xlsx", hlBitA.getHyperlink(1));
+		
+		hlBitB = (Type12)qc.getBits()[14];
+		assertEquals(3, hlBitB.getNumberOfPLCs());
+		assertEquals(3, hlBitB.getNumberOfHyperlinks());
+		
+		assertEquals(27, hlBitB.getTextStartAt(0));
+		assertEquals(37, hlBitB.getTextStartAt(1));
+		assertEquals(54, hlBitB.getTextStartAt(2));
+		assertEquals(75, hlBitB.getAllTextEndAt());
+		assertEquals("", hlBitB.getHyperlink(0));
+		assertEquals("mailto:dev@poi.apache.org?subject=HPBF", hlBitB.getHyperlink(1));
+		assertEquals("mailto:dev@poi.apache.org?subject=HPBF", hlBitB.getHyperlink(2));
+		
+		// 98 version
+		f = new File(dir, "Sample98.pub");
+		doc = new HPBFDocument(
+				new FileInputStream(f)
+		);
+		qc = doc.getQuillContents();
+		
+		hlBitA = (Type12)qc.getBits()[13];
+		assertEquals(2, hlBitA.getNumberOfPLCs());
+		assertEquals(2, hlBitA.getNumberOfHyperlinks());
+		
+		assertEquals(25, hlBitA.getTextStartAt(0));
+		assertEquals(72, hlBitA.getTextStartAt(1));
+		assertEquals(87, hlBitA.getAllTextEndAt());
+		assertEquals("http://poi.apache.org/", hlBitA.getHyperlink(0));
+		assertEquals("C:\\Documents and Settings\\Nick\\My Documents\\Booleans.xlsx", hlBitA.getHyperlink(1));
+		
+		hlBitB = (Type12)qc.getBits()[14];
+		assertEquals(3, hlBitB.getNumberOfPLCs());
+		assertEquals(3, hlBitB.getNumberOfHyperlinks());
+		
+		assertEquals(27, hlBitB.getTextStartAt(0));
+		assertEquals(37, hlBitB.getTextStartAt(1));
+		assertEquals(54, hlBitB.getTextStartAt(2));
+		assertEquals(75, hlBitB.getAllTextEndAt());
+		assertEquals("", hlBitB.getHyperlink(0));
+		assertEquals("mailto:dev@poi.apache.org?subject=HPBF", hlBitB.getHyperlink(1));
+		assertEquals("mailto:dev@poi.apache.org?subject=HPBF", hlBitB.getHyperlink(2));
+	}
 }

src/testcases/org/apache/poi/hssf/model/TestSheet.java

@@ -17,7 +17,6 @@
 
 package org.apache.poi.hssf.model;
 
-import java.io.ByteArrayInputStream;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -25,8 +24,6 @@ import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
-import org.apache.poi.hssf.eventmodel.ERFListener;
-import org.apache.poi.hssf.eventmodel.EventRecordFactory;
 import org.apache.poi.hssf.record.BOFRecord;
 import org.apache.poi.hssf.record.BlankRecord;
 import org.apache.poi.hssf.record.CellValueRecordInterface;
@@ -46,6 +43,7 @@ import org.apache.poi.hssf.record.aggregates.ColumnInfoRecordsAggregate;
 import org.apache.poi.hssf.record.aggregates.MergedCellsTable;
 import org.apache.poi.hssf.record.aggregates.PageSettingsBlock;
 import org.apache.poi.hssf.record.aggregates.RowRecordsAggregate;
+import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
 import org.apache.poi.hssf.usermodel.HSSFCell;
 import org.apache.poi.hssf.usermodel.HSSFRow;
 import org.apache.poi.hssf.usermodel.HSSFSheet;
@@ -88,15 +86,16 @@ public final class TestSheet extends TestCase {
         return result;
     }
 
-    private static final class MergedCellListener implements ERFListener {
+    private static final class MergedCellListener implements RecordVisitor {
 
         private int _count;
         public MergedCellListener() {
             _count = 0;
         }
-        public boolean processRecord(Record rec) {
+        public void visitRecord(Record r) {
-            _count++;
+            if (r instanceof MergeCellsRecord) {
-            return true;
+                _count++;
+            }
         }
         public int getCount() {
             return _count;
@@ -118,12 +117,8 @@ public final class TestSheet extends TestCase {
         assertTrue(sheet.getNumMergedRegions() == regionsToAdd);
 
         //test that the regions were spread out over the appropriate number of records
-        byte[] sheetData = new byte[sheet.getSize()];
-        sheet.serialize(0, sheetData);
         MergedCellListener mcListener = new MergedCellListener();
-        EventRecordFactory erf = new EventRecordFactory(mcListener, new short[] { MergeCellsRecord.sid, });
+        sheet.visitContainedRecords(mcListener, 0);
-//        POIFSFileSystem poifs = new POIFSFileSystem(new ByteArrayInputStream(sheetData));
-        erf.processRecords(new ByteArrayInputStream(sheetData));
         int recordsAdded    = mcListener.getCount();
         int recordsExpected = regionsToAdd/1027;
         if ((regionsToAdd % 1027) != 0)
@@ -416,6 +411,27 @@ public final class TestSheet extends TestCase {
         assertEquals(DEFAULT_IDX, xfindex);
     }
 
+    private static final class SizeCheckingRecordVisitor implements RecordVisitor {
+
+        private int _totalSize;
+        public SizeCheckingRecordVisitor() {
+            _totalSize = 0;
+        }
+        public void visitRecord(Record r) {
+
+            int estimatedSize=r.getRecordSize();
+            byte[] buf = new byte[estimatedSize];
+            int serializedSize = r.serialize(0, buf);
+            if (estimatedSize != serializedSize) {
+                throw new AssertionFailedError("serialized size mismatch for record (" 
+                        + r.getClass().getName() + ")");
+            }
+            _totalSize += estimatedSize;
+        }
+        public int getTotalSize() {
+            return _totalSize;
+        }
+    }
     /**
      * Prior to bug 45066, POI would get the estimated sheet size wrong
      * when an <tt>UncalcedRecord</tt> was present.<p/>
@@ -429,13 +445,13 @@ public final class TestSheet extends TestCase {
         records.add(createWindow2Record());
         records.add(EOFRecord.instance);
         Sheet sheet = Sheet.createSheet(records, 0, 0);
-
+        
-        int estimatedSize = sheet.getSize();
+        // The original bug was due to different logic for collecting records for sizing and 
-        int serializedSize = sheet.serialize(0, new byte[estimatedSize]);
+        // serialization. The code has since been refactored into a single method for visiting
-        if (serializedSize != estimatedSize) {
+        // all contained records.  Now this test is much less interesting
-            throw new AssertionFailedError("Identified bug 45066 b");
+        SizeCheckingRecordVisitor scrv = new SizeCheckingRecordVisitor();
-        }
+        sheet.visitContainedRecords(scrv, 0);
-        assertEquals(90, serializedSize);
+        assertEquals(90, scrv.getTotalSize());
     }
 
     /**
@@ -479,31 +495,31 @@ public final class TestSheet extends TestCase {
      * That value is found on the IndexRecord.
      */
     private static int getDbCellRecordPos(Sheet sheet) {
-        int size = sheet.getSize();
-        byte[] data = new byte[size];
-        sheet.serialize(0, data);
 
         MyIndexRecordListener myIndexListener = new MyIndexRecordListener();
-        EventRecordFactory erf = new EventRecordFactory(myIndexListener, new short[] { IndexRecord.sid, });
+        sheet.visitContainedRecords(myIndexListener, 0);
-        erf.processRecords(new ByteArrayInputStream(data));
         IndexRecord indexRecord = myIndexListener.getIndexRecord();
         int dbCellRecordPos = indexRecord.getDbcellAt(0);
         return dbCellRecordPos;
     }
 
-    private static final class MyIndexRecordListener implements ERFListener {
+    private static final class MyIndexRecordListener implements RecordVisitor {
 
         private IndexRecord _indexRecord;
         public MyIndexRecordListener() {
             // no-arg constructor
         }
-        public boolean processRecord(Record rec) {
-            _indexRecord = (IndexRecord)rec;
-            return true;
-        }
         public IndexRecord getIndexRecord() {
             return _indexRecord;
         }
+        public void visitRecord(Record r) {
+            if (r instanceof IndexRecord) {
+                if (_indexRecord != null) {
+                    throw new RuntimeException("too many index records");
+                }
+                _indexRecord = (IndexRecord)r;
+            }
+        }
     }
     
     /**
@@ -541,5 +557,23 @@ public final class TestSheet extends TestCase {
         }
         assertEquals("Informations", cell.getRichStringCellValue().getString());
     }
+    /**
+     * In 3.1, setting margins between creating first row and first cell caused an exception.
+     */
+    public void testSetMargins_bug45717() {
+        HSSFWorkbook workbook = new HSSFWorkbook();
+        HSSFSheet sheet = workbook.createSheet("Vorschauliste");
+        HSSFRow row = sheet.createRow(0);
+
+        sheet.setMargin(HSSFSheet.LeftMargin, 0.3);
+        try {
+            row.createCell((short) 0);
+        } catch (IllegalStateException e) {
+            if (e.getMessage().equals("Cannot create value records before row records exist")) {
+                throw new AssertionFailedError("Identified bug 45717");
+            }
+            throw e;
+        }
+    }
 }
 

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();
+	}
+}

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

@@ -17,23 +17,19 @@
 
 package org.apache.poi.hssf.usermodel;
 
-import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
-import java.util.Iterator;
 
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
-import org.apache.poi.hssf.eventmodel.ERFListener;
-import org.apache.poi.hssf.eventmodel.EventRecordFactory;
 import org.apache.poi.hssf.model.Workbook;
 import org.apache.poi.hssf.record.BackupRecord;
 import org.apache.poi.hssf.record.LabelSSTRecord;
 import org.apache.poi.hssf.record.Record;
-import org.apache.poi.hssf.record.aggregates.RowRecordsAggregate;
+import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.poi.ss.util.CellRangeAddress;
 import org.apache.poi.ss.util.Region;
@@ -73,10 +69,7 @@ public final class TestWorkbook extends TestCase {
      *             HSSFSheet last row or first row is incorrect.             <P>
      *
      */
-
+    public void testWriteSheetSimple() throws IOException {
-    public void testWriteSheetSimple()
-        throws IOException
-    {
         File             file = TempFile.createTempFile("testWriteSheetSimple",
                                                     ".xls");
         FileOutputStream out  = new FileOutputStream(file);
@@ -85,13 +78,10 @@ public final class TestWorkbook extends TestCase {
         HSSFRow          r    = null;
         HSSFCell         c    = null;
 
-        for (short rownum = ( short ) 0; rownum < 100; rownum++)
+        for (int rownum = 0; rownum < 100; rownum++) {
-        {
             r = s.createRow(rownum);
 
-            // r.setRowNum(( short ) rownum);
+            for (int cellnum = 0; cellnum < 50; cellnum += 2) {
-            for (short cellnum = ( short ) 0; cellnum < 50; cellnum += 2)
-            {
                 c = r.createCell(cellnum);
                 c.setCellValue(rownum * 10000 + cellnum
                                + ((( double ) rownum / 1000)
@@ -105,8 +95,6 @@ public final class TestWorkbook extends TestCase {
         sanityChecker.checkHSSFWorkbook(wb);
         assertEquals("LAST ROW == 99", 99, s.getLastRowNum());
         assertEquals("FIRST ROW == 0", 0, s.getFirstRowNum());
-
-        // assert((s.getLastRowNum() == 99));
     }
 
     /**
@@ -131,13 +119,10 @@ public final class TestWorkbook extends TestCase {
         HSSFRow          r    = null;
         HSSFCell         c    = null;
 
-        for (short rownum = ( short ) 0; rownum < 100; rownum++)
+        for (int rownum = 0; rownum < 100; rownum++) {
-        {
             r = s.createRow(rownum);
 
-            // r.setRowNum(( short ) rownum);
+            for (int cellnum = 0; cellnum < 50; cellnum += 2) {
-            for (short cellnum = ( short ) 0; cellnum < 50; cellnum += 2)
-            {
                 c = r.createCell(cellnum);
                 c.setCellValue(rownum * 10000 + cellnum
                                + ((( double ) rownum / 1000)
@@ -146,13 +131,11 @@ public final class TestWorkbook extends TestCase {
                 c.setCellValue(new HSSFRichTextString("TEST"));
             }
         }
-        for (short rownum = ( short ) 0; rownum < 25; rownum++)
+        for (int rownum = 0; rownum < 25; rownum++) {
-        {
             r = s.getRow(rownum);
             s.removeRow(r);
         }
-        for (short rownum = ( short ) 75; rownum < 100; rownum++)
+        for (int rownum = 75; rownum < 100; rownum++) {
-        {
             r = s.getRow(rownum);
             s.removeRow(r);
         }
@@ -429,12 +412,10 @@ public final class TestWorkbook extends TestCase {
         HSSFWorkbook     wb   = new HSSFWorkbook();
         HSSFSheet        s    = wb.createSheet();
 
-        for (short rownum = ( short ) 0; rownum < 100; rownum++)
+        for (int rownum = 0; rownum < 100; rownum++) {
-        {
             HSSFRow r = s.createRow(rownum);
 
-            for (short cellnum = ( short ) 0; cellnum < 50; cellnum += 2)
+            for (int cellnum = 0; cellnum < 50; cellnum += 2) {
-            {
                 HSSFCell c = r.createCell(cellnum);
                 c.setCellValue(rownum * 10000 + cellnum
                                + ((( double ) rownum / 1000)
@@ -466,13 +447,10 @@ public final class TestWorkbook extends TestCase {
     /**
      * Test the backup field gets set as expected.
      */
-
+    public void testBackupRecord() {
-    public void testBackupRecord()
+        HSSFWorkbook wb = new HSSFWorkbook();
-        throws Exception
+		wb.createSheet();
-    {
+		Workbook workbook = wb.getWorkbook();
-        HSSFWorkbook wb       = new HSSFWorkbook();
-        wb.createSheet();
-        Workbook     workbook = wb.getWorkbook();
         BackupRecord record   = workbook.getBackupRecord();
 
         assertEquals(0, record.getBackup());
@@ -480,7 +458,7 @@ public final class TestWorkbook extends TestCase {
         assertEquals(1, record.getBackup());
     }
 
-    private static final class RecordCounter implements ERFListener {
+    private static final class RecordCounter implements RecordVisitor {
         private int _count;
 
         public RecordCounter() {
@@ -489,9 +467,10 @@ public final class TestWorkbook extends TestCase {
         public int getCount() {
             return _count;
         }
-        public boolean processRecord(Record rec) {
+        public void visitRecord(Record r) {
-            _count++;
+            if (r instanceof LabelSSTRecord) {
-            return true;
+                _count++;
+            }
         }
     }
     
@@ -500,9 +479,7 @@ public final class TestWorkbook extends TestCase {
      *
      * We need to make sure only one LabelSSTRecord is produced.
      */
-    public void testRepeatingBug()
+    public void testRepeatingBug() {
-        throws Exception
-    {
         HSSFWorkbook workbook = new HSSFWorkbook();
         HSSFSheet    sheet    = workbook.createSheet("Design Variants");
         HSSFRow      row      = sheet.createRow(2);
@@ -511,12 +488,8 @@ public final class TestWorkbook extends TestCase {
         cell.setCellValue(new HSSFRichTextString("Class"));
         cell = row.createCell(2);
 
-        byte[] data = new byte[sheet.getSheet().getSize()];
-        sheet.getSheet().serialize(0, data);
         RecordCounter rc = new RecordCounter();
-        EventRecordFactory erf = new EventRecordFactory(rc, new short[] { LabelSSTRecord.sid, });
+        sheet.getSheet().visitContainedRecords(rc, 0);
-        erf.processRecords(new ByteArrayInputStream(data));
-        
         assertEquals(1, rc.getCount());
     }