Fixed serialization of multiple blank records (should get aggregated into MulBlankRecord)

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

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

@@ -37,6 +37,12 @@ public final class MulBlankRecord extends StandardRecord {
     private short[]           field_3_xfs;
     private short             field_4_last_col;
 
+    public MulBlankRecord(int row, int firstCol, short[] xfs) {
+        field_1_row = row;
+        field_2_first_col = (short)firstCol;
+        field_3_xfs = xfs;
+        field_4_last_col = (short) (firstCol + xfs.length - 1);
+    }
 
     /**
      * get the row number of the cells this represents
@@ -127,9 +133,17 @@ public final class MulBlankRecord extends StandardRecord {
     }
 
     public void serialize(LittleEndianOutput out) {
-        throw new RecordFormatException( "Sorry, you can't serialize MulBlank in this release");
+        out.writeShort(field_1_row);
+        out.writeShort(field_2_first_col);
+        int nItems = field_3_xfs.length;
+        for (int i = 0; i < nItems; i++) {
+            out.writeShort(field_3_xfs[i]);
+        }
+        out.writeShort(field_4_last_col);
     }
+
     protected int getDataSize() {
-        throw new RecordFormatException( "Sorry, you can't serialize MulBlank in this release");
+        // 3 short fields + array of shorts
+        return 6 + field_3_xfs.length * 2; 
     }
 }

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

@@ -46,7 +46,7 @@ import org.apache.poi.hssf.record.pivottable.*;
 public final class RecordFactory {
 	private static final int NUM_RECORDS = 512;
 
-	private static final Class[] CONSTRUCTOR_ARGS = { RecordInputStream.class, };
+	private static final Class<?>[] CONSTRUCTOR_ARGS = { RecordInputStream.class, };
 
 	/**
 	 * contains the classes for all the records we want to parse.<br/>
@@ -189,7 +189,7 @@ public final class RecordFactory {
 	/**
 	 * cache of the recordsToMap();
 	 */
-	private static Map recordsMap  = recordsToMap(recordClasses);
+	private static Map<Short, Constructor<? extends Record>> recordsMap  = recordsToMap(recordClasses);
 
 	private static short[] _allKnownRecordSIDs;
 	
@@ -210,21 +210,18 @@ public final class RecordFactory {
 		if (record instanceof MulRKRecord) {
 			return convertRKRecords((MulRKRecord)record);
 		}
-		if (record instanceof MulBlankRecord) {
-			return convertMulBlankRecords((MulBlankRecord)record);
-		}
 		return new Record[] { record, };
 	}
 	
 	private static Record createSingleRecord(RecordInputStream in) {
-		Constructor constructor = (Constructor) recordsMap.get(new Short(in.getSid()));
+		Constructor<? extends Record> constructor = recordsMap.get(new Short(in.getSid()));
 
 		if (constructor == null) {
 			return new UnknownRecord(in);
 		}
 		
 		try {
-			return (Record) constructor.newInstance(new Object[] { in });
+			return constructor.newInstance(new Object[] { in });
 		} catch (InvocationTargetException e) {
 			throw new RecordFormatException("Unable to construct record instance" , e.getTargetException());
 		} catch (IllegalArgumentException e) {
@@ -268,23 +265,6 @@ public final class RecordFactory {
 		return mulRecs;
 	}
 
-	/**
-	 * Converts a {@link MulBlankRecord} into an equivalent array of {@link BlankRecord}s
-	 */
-	private static BlankRecord[] convertMulBlankRecords(MulBlankRecord mb) {
-
-		BlankRecord[] mulRecs = new BlankRecord[mb.getNumColumns()];
-		for (int k = 0; k < mb.getNumColumns(); k++) {
-			BlankRecord br = new BlankRecord();
-
-			br.setColumn((short) (k + mb.getFirstColumn()));
-			br.setRow(mb.getRow());
-			br.setXFIndex(mb.getXFAt(k));
-			mulRecs[k] = br;
-		}
-		return mulRecs;
-	}
-
 	/**
 	 * @return an array of all the SIDS for all known records
 	 */
@@ -293,8 +273,8 @@ public final class RecordFactory {
 			short[] results = new short[ recordsMap.size() ];
 			int i = 0;
 
-			for (Iterator iterator = recordsMap.keySet().iterator(); iterator.hasNext(); ) {
+			for (Iterator<Short> iterator = recordsMap.keySet().iterator(); iterator.hasNext(); ) {
-				Short sid = (Short) iterator.next();
+				Short sid = iterator.next();
 
 				results[i++] = sid.shortValue();
 			}
@@ -330,7 +310,7 @@ public final class RecordFactory {
 			short sid;
 			Constructor<? extends Record> constructor;
 			try {
-				sid		 = recClass.getField("sid").getShort(null);
+				sid = recClass.getField("sid").getShort(null);
 				constructor = recClass.getConstructor(CONSTRUCTOR_ARGS);
 			} catch (Exception illegalArgumentException) {
 				throw new RecordFormatException(
@@ -338,7 +318,7 @@ public final class RecordFactory {
 			}
 			Short key = new Short(sid);
 			if (result.containsKey(key)) {
-				Class prev = result.get(key).getDeclaringClass();
+				Class<? extends Record> prev = result.get(key).getDeclaringClass();
 				throw new RuntimeException("duplicate record sid 0x" + Integer.toHexString(sid).toUpperCase()
 						+ " for classes (" + recClass.getName() + ") and (" + prev.getName() + ")");
 			}
@@ -356,7 +336,7 @@ public final class RecordFactory {
 	 *
 	 * @exception RecordFormatException on error processing the InputStream
 	 */
-	public static List createRecords(InputStream in) throws RecordFormatException {
+	public static List<Record> createRecords(InputStream in) throws RecordFormatException {
 
 		List<Record> records = new ArrayList<Record>(NUM_RECORDS);
 
@@ -384,10 +364,6 @@ public final class RecordFactory {
 				addAll(records, convertRKRecords((MulRKRecord)record));
 				continue;
 			}
-			if (record instanceof MulBlankRecord) {
-				addAll(records, convertMulBlankRecords((MulBlankRecord)record));
-				continue;
-			}
 
 			if (record.getSid() == DrawingGroupRecord.sid
 				   && lastRecord instanceof DrawingGroupRecord) {

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

@@ -32,6 +32,7 @@ import org.apache.poi.hssf.record.DimensionsRecord;
 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.MulBlankRecord;
 import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.RowRecord;
 import org.apache.poi.hssf.record.SharedFormulaRecord;
@@ -88,6 +89,10 @@ public final class RowRecordsAggregate extends RecordAggregate {
 				}
 				continue;
 			}
+			if (rec instanceof MulBlankRecord) {
+				_valuesAgg.addMultipleBlanks((MulBlankRecord) rec);
+				continue;
+			}
 			if (!(rec instanceof CellValueRecordInterface)) {
 				throw new RuntimeException("Unexpected record type (" + rec.getClass().getName() + ")");
 			}

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

@@ -18,12 +18,13 @@
 package org.apache.poi.hssf.record.aggregates;
 
 import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
 
 import org.apache.poi.hssf.model.RecordStream;
+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.MulBlankRecord;
 import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.RecordBase;
 import org.apache.poi.hssf.record.StringRecord;
@@ -41,14 +42,20 @@ import org.apache.poi.hssf.record.formula.Ptg;
  */
 public final class ValueRecordsAggregate {
 	private static final int MAX_ROW_INDEX = 0XFFFF;
-	private int firstcell = -1;
+	private static final int INDEX_NOT_SET = -1;
-	private int lastcell  = -1;
+	private int firstcell = INDEX_NOT_SET;
+	private int lastcell  = INDEX_NOT_SET;
 	private CellValueRecordInterface[][] records;
 
 	/** Creates a new instance of ValueRecordsAggregate */
 
 	public ValueRecordsAggregate() {
-		records = new CellValueRecordInterface[30][]; // We start with 30 Rows.
+		this(INDEX_NOT_SET, INDEX_NOT_SET, new CellValueRecordInterface[30][]); // We start with 30 Rows.
+	}
+	private ValueRecordsAggregate(int firstCellIx, int lastCellIx, CellValueRecordInterface[][] pRecords) {
+		firstcell = firstCellIx;
+		lastcell = lastCellIx;
+		records = pRecords;
 	}
 
 	public void insertCell(CellValueRecordInterface cell) {
@@ -82,10 +89,10 @@ public final class ValueRecordsAggregate {
 		}
 		rowCells[column] = cell;
 
-		if ((column < firstcell) || (firstcell == -1)) {
+		if (column < firstcell || firstcell == INDEX_NOT_SET) {
 			firstcell = column;
 		}
-		if ((column > lastcell) || (lastcell == -1)) {
+		if (column > lastcell || lastcell == INDEX_NOT_SET) {
 			lastcell = column;
 		}
 	}
@@ -115,11 +122,11 @@ public final class ValueRecordsAggregate {
 					+ " is outside the allowable range (0.." +MAX_ROW_INDEX + ")");
 		}
 		if (rowIndex >= records.length) {
-			// this can happen when the client code has created a row, 
+			// this can happen when the client code has created a row,
-			// and then removes/replaces it before adding any cells. (see bug 46312) 
+			// and then removes/replaces it before adding any cells. (see bug 46312)
 			return;
-		} 
+		}
-		
+
 		records[rowIndex] = null;
 	}
 
@@ -146,6 +153,17 @@ public final class ValueRecordsAggregate {
 		return lastcell;
 	}
 
+	public void addMultipleBlanks(MulBlankRecord mbr) {
+		for (int j = 0; j < mbr.getNumColumns(); j++) {
+			BlankRecord br = new BlankRecord();
+
+			br.setColumn(( short ) (j + mbr.getFirstColumn()));
+			br.setRow(mbr.getRow());
+			br.setXFIndex(mbr.getXFAt(j));
+			insertCell(br);
+		}
+	}
+
 	/**
 	 * Processes a single cell value record
 	 * @param sfh used to resolve any shared-formulas/arrays/tables for the current sheet
@@ -155,7 +173,7 @@ public final class ValueRecordsAggregate {
 			FormulaRecord formulaRec = (FormulaRecord)rec;
 			// read optional cached text value
 			StringRecord cachedText;
-			Class nextClass = rs.peekNextClass();
+			Class<? extends Record> nextClass = rs.peekNextClass();
 			if (nextClass == StringRecord.class) {
 				cachedText = (StringRecord) rs.getNext();
 			} else {
@@ -171,19 +189,11 @@ public final class ValueRecordsAggregate {
 	 *  that are attached to the rows in the range specified.
 	 */
 	public int getRowCellBlockSize(int startRow, int endRow) {
-		MyIterator itr = new MyIterator(records, startRow, endRow);
+		int result = 0;
-		int size = 0;
+		for(int rowIx=startRow; rowIx<=endRow && rowIx<records.length; rowIx++) {
-		while (itr.hasNext()) {
+			result += getRowSerializedSize(records[rowIx]);
-			CellValueRecordInterface cell = (CellValueRecordInterface) itr.next();
-			int row = cell.getRow();
-			if (row > endRow) {
-				break;
-			}
-			if ((row >= startRow) && (row <= endRow)) {
-				size += ((RecordBase) cell).getRecordSize();
-			}
 		}
-		return size;
+		return result;
 	}
 
 	/** Returns true if the row has cells attached to it */
@@ -191,7 +201,7 @@ public final class ValueRecordsAggregate {
 		if (row >= records.length) {
 			return false;
 		}
-		CellValueRecordInterface[] rowCells=records[row]; 
+		CellValueRecordInterface[] rowCells=records[row];
 		if(rowCells==null) return false;
 		for(int col=0;col<rowCells.length;col++) {
 			if(rowCells[col]!=null) return true;
@@ -199,42 +209,79 @@ public final class ValueRecordsAggregate {
 		return false;
 	}
 
-	/** Serializes the cells that are allocated to a certain row range*/
+	private static int getRowSerializedSize(CellValueRecordInterface[] rowCells) {
-	public int serializeCellRow(final int row, int offset, byte [] data)
+		if(rowCells == null) {
-	{
+			return 0;
-	  MyIterator itr = new MyIterator(records, row, row);
-		int	  pos = offset;
-
-		while (itr.hasNext())
-		{
-			CellValueRecordInterface cell = (CellValueRecordInterface)itr.next();
-			if (cell.getRow() != row)
-			  break;
-			pos += (( RecordBase ) cell).serialize(pos, data);
 		}
-		return pos - offset;
+		int result = 0;
+		for (int i = 0; i < rowCells.length; i++) {
+			RecordBase cvr = (RecordBase) rowCells[i];
+			if(cvr == null) {
+				continue;
+			}
+			int nBlank = countBlanks(rowCells, i);
+			if (nBlank > 1) {
+				result += (10 + 2*nBlank);
+				i+=nBlank-1;
+			} else {
+				result += cvr.getRecordSize();
+			}
+		}
+		return result;
 	}
 
 	public void visitCellsForRow(int rowIndex, RecordVisitor rv) {
 
-		CellValueRecordInterface[] cellRecs = records[rowIndex];
+		CellValueRecordInterface[] rowCells = records[rowIndex];
-		if (cellRecs != null) {
+		if(rowCells == null) {
-			for (int i = 0; i < cellRecs.length; i++) {
+			throw new IllegalArgumentException("Row [" + rowIndex + "] is empty");
-				CellValueRecordInterface cvr = cellRecs[i];
+		}
-				if (cvr == null) {
+
-					continue;
+
-				}
+		for (int i = 0; i < rowCells.length; i++) {
-				if (cvr instanceof RecordAggregate) {
+			RecordBase cvr = (RecordBase) rowCells[i];
-					RecordAggregate agg = (RecordAggregate) cvr;
+			if(cvr == null) {
-					agg.visitContainedRecords(rv);
+				continue;
-				} else {
+			}
-					Record rec = (Record) cvr;
+			int nBlank = countBlanks(rowCells, i);
-					rv.visitRecord(rec);
+			if (nBlank > 1) {
-				}
+				rv.visitRecord(createMBR(rowCells, i, nBlank));
+				i+=nBlank-1;
+			} else if (cvr instanceof RecordAggregate) {
+				RecordAggregate agg = (RecordAggregate) cvr;
+				agg.visitContainedRecords(rv);
+			} else {
+				rv.visitRecord((Record) cvr);
 			}
 		}
 	}
 
+	/**
+	 * @return the number of <em>consecutive</em> {@link BlankRecord}s in the specified row
+	 * starting from startIx.
+	 */
+	private static int countBlanks(CellValueRecordInterface[] rowCellValues, int startIx) {
+		int i = startIx;
+		while(i < rowCellValues.length) {
+			CellValueRecordInterface cvr = rowCellValues[i];
+			if (!(cvr instanceof BlankRecord)) {
+				break;
+			}
+			i++;
+		}
+		return i - startIx;
+	}
+
+	private MulBlankRecord createMBR(CellValueRecordInterface[] cellValues, int startIx, int nBlank) {
+
+		short[] xfs = new short[nBlank];
+		for (int i = 0; i < xfs.length; i++) {
+			xfs[i] = ((BlankRecord)cellValues[startIx + i]).getXFIndex();
+		}
+		int rowIx = cellValues[startIx].getRow();
+		return new MulBlankRecord(rowIx, startIx, xfs);
+	}
+
 	public void updateFormulasAfterRowShift(FormulaShifter shifter, int currentExternSheetIndex) {
 		for (int i = 0; i < records.length; i++) {
 			CellValueRecordInterface[] rowCells = records[i];
@@ -254,16 +301,20 @@ public final class ValueRecordsAggregate {
 		}
 	}
 
+	/**
+	 * Gets all the cell records contained in this aggregate. 
+	 * Note {@link BlankRecord}s appear separate (not in {@link MulBlankRecord}s).
+	 */
 	public CellValueRecordInterface[] getValueRecords() {
 		List<CellValueRecordInterface> temp = new ArrayList<CellValueRecordInterface>();
 
-		for (int i = 0; i < records.length; i++) {
+		for (int rowIx = 0; rowIx < records.length; rowIx++) {
-			CellValueRecordInterface[] rowCells = records[i];
+			CellValueRecordInterface[] rowCells = records[rowIx];
 			if (rowCells == null) {
 				continue;
 			}
-			for (int j = 0; j < rowCells.length; j++) {
+			for (int colIx = 0; colIx < rowCells.length; colIx++) {
-				CellValueRecordInterface cell = rowCells[j];
+				CellValueRecordInterface cell = rowCells[colIx];
 				if (cell != null) {
 					temp.add(cell);
 				}
@@ -274,57 +325,8 @@ public final class ValueRecordsAggregate {
 		temp.toArray(result);
 		return result;
 	}
-	public Iterator getIterator() {
-		return new MyIterator(records);
-	}
-
-  private static final class MyIterator implements Iterator {
-		private final CellValueRecordInterface[][] records;
-		private short nextColumn = -1;
-		private int nextRow, lastRow;
-
-		public MyIterator(CellValueRecordInterface[][] pRecords) {
-			this(pRecords, 0, pRecords.length - 1);
-		}
-
-		public MyIterator(CellValueRecordInterface[][] pRecords, int firstRow, int lastRow) {
-			records = pRecords;
-			this.nextRow = firstRow;
-			this.lastRow = lastRow;
-			findNext();
-		}
-
-		public boolean hasNext() {
-			return nextRow <= lastRow;
-		}
-
-		public Object next() {
-			Object o = records[nextRow][nextColumn];
-			findNext();
-			return o;
-		}
-
-		public void remove() {
-			throw new UnsupportedOperationException("gibt's noch nicht");
-		}
-
-		private void findNext() {
-			nextColumn++;
-			for (; nextRow <= lastRow; nextRow++) {
-				// previously this threw array out of bounds...
-				CellValueRecordInterface[] rowCells = (nextRow < records.length) ? records[nextRow]
-						: null;
-				if (rowCells == null) { // This row is empty
-					nextColumn = 0;
-					continue;
-				}
-				for (; nextColumn < rowCells.length; nextColumn++) {
-					if (rowCells[nextColumn] != null)
-						return;
-				}
-				nextColumn = 0;
-			}
-		}
 
+	public Object clone() {
+		throw new RuntimeException("clone() should not be called.  ValueRecordsAggregate should be copied via Sheet.cloneSheet()");
 	}
 }

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

@@ -20,7 +20,7 @@ package org.apache.poi.hssf.record.aggregates;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
-import java.util.Iterator;
+import java.util.Arrays;
 import java.util.List;
 import java.util.zip.CRC32;
 
@@ -33,13 +33,15 @@ 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.MulBlankRecord;
 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.record.aggregates.RecordAggregate.RecordVisitor;
 import org.apache.poi.hssf.usermodel.HSSFRow;
 import org.apache.poi.hssf.usermodel.HSSFSheet;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+import org.apache.poi.util.HexRead;
 
 /**
  * Tests for {@link ValueRecordsAggregate}
@@ -59,16 +61,16 @@ public final class TestValueRecordsAggregate extends TestCase {
 		records.add(new WindowTwoRecord());
 
 		constructValueRecord(records);
-		Iterator iterator = valueRecord.getIterator();
+		CellValueRecordInterface[] cvrs = valueRecord.getValueRecords();
-		RecordBase record = (RecordBase) iterator.next();
+		//Ensure that the SharedFormulaRecord has been converted
+		assertEquals(1, cvrs.length);
+
+		CellValueRecordInterface record = cvrs[0];
 		assertNotNull( "Row contains a value", record );
 		assertTrue( "First record is a FormulaRecordsAggregate", ( record instanceof FormulaRecordAggregate ) );
-		//Ensure that the SharedFormulaRecord has been converted
-		assertFalse( "SharedFormulaRecord is null", iterator.hasNext() );
-
 	}
 
-	private void constructValueRecord(List records) {
+	private void constructValueRecord(List<Record> records) {
 		RowBlocksReader rbr = new RowBlocksReader(new RecordStream(records, 0));
 		SharedValueManager sfrh = rbr.getSharedFormulaManager();
 		RecordStream rs = rbr.getPlainRecordStream();
@@ -78,7 +80,7 @@ public final class TestValueRecordsAggregate extends TestCase {
 		}
 	}
 
-	private static List testData() {
+	private static List<Record> testData() {
 		List<Record> records = new ArrayList<Record>();
 		FormulaRecord formulaRecord = new FormulaRecord();
 		BlankRecord blankRecord = new BlankRecord();
@@ -93,13 +95,13 @@ public final class TestValueRecordsAggregate extends TestCase {
 	}
 
 	public void testInsertCell() {
-		Iterator iterator = valueRecord.getIterator();
+		CellValueRecordInterface[] cvrs = valueRecord.getValueRecords();
-		assertFalse( iterator.hasNext() );
+		assertEquals(0, cvrs.length);
 
 		BlankRecord blankRecord = newBlankRecord();
 		valueRecord.insertCell( blankRecord );
-		iterator = valueRecord.getIterator();
+		cvrs = valueRecord.getValueRecords();
-		assertTrue( iterator.hasNext() );
+		assertEquals(1, cvrs.length);
 	}
 
 	public void testRemoveCell() {
@@ -107,8 +109,8 @@ public final class TestValueRecordsAggregate extends TestCase {
 		valueRecord.insertCell( blankRecord1 );
 		BlankRecord blankRecord2 = newBlankRecord();
 		valueRecord.removeCell( blankRecord2 );
-		Iterator iterator = valueRecord.getIterator();
+		CellValueRecordInterface[] cvrs = valueRecord.getValueRecords();
-		assertFalse( iterator.hasNext() );
+		assertEquals(0, cvrs.length);
 
 		// removing an already empty cell just falls through
 		valueRecord.removeCell( blankRecord2 );
@@ -148,36 +150,46 @@ public final class TestValueRecordsAggregate extends TestCase {
 
 	}
 
-	public void testSerialize() {
+
-		byte[] actualArray = new byte[36];
+	private static final class SerializerVisitor implements RecordVisitor {
-		byte[] expectedArray = new byte[]
+		private final byte[] _buf;
-		{
+		private int _writeIndex;
-			(byte)0x06, (byte)0x00, (byte)0x16, (byte)0x00,
+		public SerializerVisitor(byte[] buf) {
-			(byte)0x01, (byte)0x00, (byte)0x01, (byte)0x00,
+			_buf = buf;
-			(byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+			_writeIndex = 0;
-			(byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+
-			(byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+		}
-			(byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00,
+		public void visitRecord(Record r) {
-			(byte)0x00, (byte)0x00, (byte)0x01, (byte)0x02,
+			r.serialize(_writeIndex, _buf);
-			(byte)0x06, (byte)0x00, (byte)0x02, (byte)0x00,
+			_writeIndex += r.getRecordSize();
-			(byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00,
+		}
-		};
+		public int getWriteIndex() {
-		List records = testData();
+			return _writeIndex;
-		constructValueRecord(records);
+		}
-		int bytesWritten = valueRecord.serializeCellRow(1, 0, actualArray );
-		bytesWritten += valueRecord.serializeCellRow(2, bytesWritten, actualArray );
-		assertEquals( 36, bytesWritten );
-		for (int i = 0; i < 36; i++)
-			assertEquals( expectedArray[i], actualArray[i] );
 	}
 
-	private static BlankRecord newBlankRecord()
+	public void testSerialize() {
-	{
+		byte[] expectedArray = HexRead.readFromString(""
+				+ "06 00 16 00 " // Formula
+				+ "01 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 "
+				+ "01 02 06 00 " // Blank
+				+ "02 00 02 00 00 00");
+		byte[] actualArray = new byte[expectedArray.length];
+		List<Record> records = testData();
+		constructValueRecord(records);
+
+		SerializerVisitor sv = new SerializerVisitor(actualArray);
+		valueRecord.visitCellsForRow(1, sv);
+		valueRecord.visitCellsForRow(2, sv);
+		assertEquals(actualArray.length, sv.getWriteIndex());
+		assertTrue(Arrays.equals(expectedArray, actualArray));
+	}
+
+	private static BlankRecord newBlankRecord() {
 		return newBlankRecord( 2, 2 );
 	}
 
-	private static BlankRecord newBlankRecord( int col, int row)
+	private static BlankRecord newBlankRecord(int col, int row) {
-	{
 		BlankRecord blankRecord = new BlankRecord();
 		blankRecord.setRow( row );
 		blankRecord.setColumn( (short) col );
@@ -185,19 +197,19 @@ public final class TestValueRecordsAggregate extends TestCase {
 	}
 
 	/**
-	 * Sometimes the 'shared formula' flag (<tt>FormulaRecord.isSharedFormula()</tt>) is set when 
+	 * Sometimes the 'shared formula' flag (<tt>FormulaRecord.isSharedFormula()</tt>) is set when
 	 * there is no corresponding SharedFormulaRecord available. SharedFormulaRecord definitions do
-	 * not span multiple sheets.  They are are only defined within a sheet, and thus they do not 
+	 * not span multiple sheets.  They are are only defined within a sheet, and thus they do not
 	 * have a sheet index field (only row and column range fields).<br/>
-	 * So it is important that the code which locates the SharedFormulaRecord for each 
+	 * So it is important that the code which locates the SharedFormulaRecord for each
-	 * FormulaRecord does not allow matches across sheets.</br> 
+	 * FormulaRecord does not allow matches across sheets.</br>
-	 * 
+	 *
-	 * Prior to bugzilla 44449 (Feb 2008), POI <tt>ValueRecordsAggregate.construct(int, List)</tt> 
+	 * Prior to bugzilla 44449 (Feb 2008), POI <tt>ValueRecordsAggregate.construct(int, List)</tt>
 	 * allowed <tt>SharedFormulaRecord</tt>s to be erroneously used across sheets.  That incorrect
 	 * behaviour is shown by this test.<p/>
-	 * 
+	 *
 	 * <b>Notes on how to produce the test spreadsheet</b>:</p>
-	 * The setup for this test (AbnormalSharedFormulaFlag.xls) is rather fragile, insomuchas 
+	 * The setup for this test (AbnormalSharedFormulaFlag.xls) is rather fragile, insomuchas
 	 * re-saving the file (either with Excel or POI) clears the flag.<br/>
 	 * <ol>
 	 * <li>A new spreadsheet was created in Excel (File | New | Blank Workbook).</li>
@@ -207,15 +219,15 @@ public final class TestValueRecordsAggregate extends TestCase {
 	 * <li>Four rows on Sheet1 "5" through "8" were deleted ('delete rows' alt-E D, not 'clear' Del).</li>
 	 * <li>The spreadsheet was saved as AbnormalSharedFormulaFlag.xls.</li>
 	 * </ol>
-	 * Prior to the row delete action the spreadsheet has two <tt>SharedFormulaRecord</tt>s. One 
+	 * Prior to the row delete action the spreadsheet has two <tt>SharedFormulaRecord</tt>s. One
 	 * for each sheet. To expose the bug, the shared formulas have been made to overlap.<br/>
-	 * The row delete action (as described here) seems to to delete the 
+	 * The row delete action (as described here) seems to to delete the
 	 * <tt>SharedFormulaRecord</tt> from Sheet1 (but not clear the 'shared formula' flags.<br/>
-	 * There are other variations on this theme to create the same effect.  
+	 * There are other variations on this theme to create the same effect.
-	 * 
+	 *
 	 */
 	public void testSpuriousSharedFormulaFlag() {
-		
+
 		long actualCRC = getFileCRC(HSSFTestDataSamples.openSampleFileStream(ABNORMAL_SHARED_FORMULA_FLAG_TEST_FILE));
 		long expectedCRC = 2277445406L;
 		if(actualCRC != expectedCRC) {
@@ -223,17 +235,17 @@ public final class TestValueRecordsAggregate extends TestCase {
 			throw failUnexpectedTestFileChange();
 		}
 		HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook(ABNORMAL_SHARED_FORMULA_FLAG_TEST_FILE);
-		
+
 		HSSFSheet s = wb.getSheetAt(0); // Sheet1
-		
+
 		String cellFormula;
 		cellFormula = getFormulaFromFirstCell(s, 0); // row "1"
 		// the problem is not observable in the first row of the shared formula
 		if(!cellFormula.equals("\"first formula\"")) {
 			throw new RuntimeException("Something else wrong with this test case");
 		}
-		
+
-		// but the problem is observable in rows 2,3,4 
+		// but the problem is observable in rows 2,3,4
 		cellFormula = getFormulaFromFirstCell(s, 1); // row "2"
 		if(cellFormula.equals("\"second formula\"")) {
 			throw new AssertionFailedError("found bug 44449 (Wrong SharedFormulaRecord was used).");
@@ -260,8 +272,8 @@ public final class TestValueRecordsAggregate extends TestCase {
 		// A breakpoint in ValueRecordsAggregate.handleMissingSharedFormulaRecord(FormulaRecord)
 		// should get hit during parsing of Sheet1.
 		// If the test spreadsheet is created as directed, this condition should occur.
-		// It is easy to upset the test spreadsheet (for example re-saving will destroy the 
+		// It is easy to upset the test spreadsheet (for example re-saving will destroy the
-		// peculiar condition we are testing for). 
+		// peculiar condition we are testing for).
 		throw new RuntimeException(msg);
 	}
 
@@ -283,13 +295,13 @@ public final class TestValueRecordsAggregate extends TestCase {
 		} catch (IOException e) {
 			throw new RuntimeException(e);
 		}
-		
+
 		return crc.getValue();
 	}
 	public void testRemoveNewRow_bug46312() {
 		// To make bug occur, rowIndex needs to be >= ValueRecordsAggregate.records.length
 		int rowIndex = 30;
-		
+
 		ValueRecordsAggregate vra = new ValueRecordsAggregate();
 		try {
 			vra.removeAllCellsValuesForRow(rowIndex);
@@ -315,4 +327,80 @@ public final class TestValueRecordsAggregate extends TestCase {
 			}
 		}
 	}
+
+	/**
+	 * Tests various manipulations of blank cells, to make sure that {@link MulBlankRecord}s
+	 * are use appropriately
+	 */
+	public void testMultipleBlanks() {
+		BlankRecord brA2 = newBlankRecord(0, 1);
+		BlankRecord brB2 = newBlankRecord(1, 1);
+		BlankRecord brC2 = newBlankRecord(2, 1);
+		BlankRecord brD2 = newBlankRecord(3, 1);
+		BlankRecord brE2 = newBlankRecord(4, 1);
+		BlankRecord brB3 = newBlankRecord(1, 2);
+		BlankRecord brC3 = newBlankRecord(2, 2);
+
+		valueRecord.insertCell(brA2);
+		valueRecord.insertCell(brB2);
+		valueRecord.insertCell(brD2);
+		confirmMulBlank(3, 1, 1);
+
+		valueRecord.insertCell(brC3);
+		confirmMulBlank(4, 1, 2);
+
+		valueRecord.insertCell(brB3);
+		valueRecord.insertCell(brE2);
+		confirmMulBlank(6, 3, 0);
+
+		valueRecord.insertCell(brC2);
+		confirmMulBlank(7, 2, 0);
+
+		valueRecord.removeCell(brA2);
+		confirmMulBlank(6, 2, 0);
+
+		valueRecord.removeCell(brC2);
+		confirmMulBlank(5, 2, 1);
+
+		valueRecord.removeCell(brC3);
+		confirmMulBlank(4, 1, 2);
+	}
+
+	private void confirmMulBlank(int expectedTotalBlankCells,
+			int expectedNumberOfMulBlankRecords, int expectedNumberOfSingleBlankRecords) {
+		// assumed row ranges set-up by caller:
+		final int firstRow = 1;
+		final int lastRow = 2;
+
+
+		final class BlankStats {
+			public int countBlankCells;
+			public int countMulBlankRecords;
+			public int countSingleBlankRecords;
+		}
+
+		final BlankStats bs = new BlankStats();
+		RecordVisitor rv = new RecordVisitor() {
+
+			public void visitRecord(Record r) {
+				if (r instanceof MulBlankRecord) {
+					MulBlankRecord mbr = (MulBlankRecord) r;
+					bs.countMulBlankRecords++;
+					bs.countBlankCells += mbr.getNumColumns();
+				} else if (r instanceof BlankRecord) {
+					bs.countSingleBlankRecords++;
+					bs.countBlankCells++;
+				}
+			}
+		};
+
+		for (int rowIx = firstRow; rowIx <=lastRow; rowIx++) {
+			if (valueRecord.rowHasCells(rowIx)) {
+				valueRecord.visitCellsForRow(rowIx, rv);
+			}
+		}
+		assertEquals(expectedTotalBlankCells, bs.countBlankCells);
+		assertEquals(expectedNumberOfMulBlankRecords, bs.countMulBlankRecords);
+		assertEquals(expectedNumberOfSingleBlankRecords, bs.countSingleBlankRecords);
+	}
 }