From 771369c409abd427a7178ad5f82062b23d5f20c7 Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Sat, 7 Feb 2009 07:39:23 +0000 Subject: [PATCH] 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 --- .../poi/hssf/record/MulBlankRecord.java | 18 +- .../apache/poi/hssf/record/RecordFactory.java | 42 +--- .../aggregates/RowRecordsAggregate.java | 5 + .../aggregates/ValueRecordsAggregate.java | 214 +++++++++--------- .../aggregates/TestValueRecordsAggregate.java | 210 ++++++++++++----- 5 files changed, 287 insertions(+), 202 deletions(-) diff --git a/src/java/org/apache/poi/hssf/record/MulBlankRecord.java b/src/java/org/apache/poi/hssf/record/MulBlankRecord.java index 009148ea3..20a061a02 100644 --- a/src/java/org/apache/poi/hssf/record/MulBlankRecord.java +++ b/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; } } diff --git a/src/java/org/apache/poi/hssf/record/RecordFactory.java b/src/java/org/apache/poi/hssf/record/RecordFactory.java index 7df423676..b50156dbc 100644 --- a/src/java/org/apache/poi/hssf/record/RecordFactory.java +++ b/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.
@@ -189,7 +189,7 @@ public final class RecordFactory { /** * cache of the recordsToMap(); */ - private static Map recordsMap = recordsToMap(recordClasses); + private static Map> 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 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(); ) { - Short sid = (Short) iterator.next(); + for (Iterator iterator = recordsMap.keySet().iterator(); iterator.hasNext(); ) { + Short sid = iterator.next(); results[i++] = sid.shortValue(); } @@ -330,7 +310,7 @@ public final class RecordFactory { short sid; Constructor 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 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 createRecords(InputStream in) throws RecordFormatException { List records = new ArrayList(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) { diff --git a/src/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java b/src/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java index 35b766ed5..f3601f6f9 100644 --- a/src/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java +++ b/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() + ")"); } diff --git a/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java b/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java index 6ee871a7f..17de37ed1 100644 --- a/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java +++ b/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 int lastcell = -1; + private static final int INDEX_NOT_SET = -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, - // and then removes/replaces it before adding any cells. (see bug 46312) + // this can happen when the client code has created a row, + // 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 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 size = 0; - while (itr.hasNext()) { - CellValueRecordInterface cell = (CellValueRecordInterface) itr.next(); - int row = cell.getRow(); - if (row > endRow) { - break; - } - if ((row >= startRow) && (row <= endRow)) { - size += ((RecordBase) cell).getRecordSize(); - } + int result = 0; + for(int rowIx=startRow; rowIx<=endRow && rowIx= records.length) { return false; } - CellValueRecordInterface[] rowCells=records[row]; + CellValueRecordInterface[] rowCells=records[row]; if(rowCells==null) return false; for(int col=0;col 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]; - if (cellRecs != null) { - for (int i = 0; i < cellRecs.length; i++) { - CellValueRecordInterface cvr = cellRecs[i]; - if (cvr == null) { - continue; - } - if (cvr instanceof RecordAggregate) { - RecordAggregate agg = (RecordAggregate) cvr; - agg.visitContainedRecords(rv); - } else { - Record rec = (Record) cvr; - rv.visitRecord(rec); - } + CellValueRecordInterface[] rowCells = records[rowIndex]; + if(rowCells == null) { + throw new IllegalArgumentException("Row [" + rowIndex + "] is empty"); + } + + + 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) { + 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 consecutive {@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 temp = new ArrayList(); - for (int i = 0; i < records.length; i++) { - CellValueRecordInterface[] rowCells = records[i]; + for (int rowIx = 0; rowIx < records.length; rowIx++) { + CellValueRecordInterface[] rowCells = records[rowIx]; if (rowCells == null) { continue; } - for (int j = 0; j < rowCells.length; j++) { - CellValueRecordInterface cell = rowCells[j]; + for (int colIx = 0; colIx < rowCells.length; colIx++) { + 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()"); } } diff --git a/src/testcases/org/apache/poi/hssf/record/aggregates/TestValueRecordsAggregate.java b/src/testcases/org/apache/poi/hssf/record/aggregates/TestValueRecordsAggregate.java index af119f29b..a8240995e 100755 --- a/src/testcases/org/apache/poi/hssf/record/aggregates/TestValueRecordsAggregate.java +++ b/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(); - RecordBase record = (RecordBase) iterator.next(); + CellValueRecordInterface[] cvrs = valueRecord.getValueRecords(); + //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 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 testData() { List records = new ArrayList(); 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(); - assertFalse( iterator.hasNext() ); + CellValueRecordInterface[] cvrs = valueRecord.getValueRecords(); + assertEquals(0, cvrs.length); BlankRecord blankRecord = newBlankRecord(); valueRecord.insertCell( blankRecord ); - iterator = valueRecord.getIterator(); - assertTrue( iterator.hasNext() ); + cvrs = valueRecord.getValueRecords(); + 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(); - assertFalse( iterator.hasNext() ); + CellValueRecordInterface[] cvrs = valueRecord.getValueRecords(); + 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]; - byte[] expectedArray = new byte[] - { - (byte)0x06, (byte)0x00, (byte)0x16, (byte)0x00, - (byte)0x01, (byte)0x00, (byte)0x01, (byte)0x00, - (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, - (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, - (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, - (byte)0x00, (byte)0x00, (byte)0x00, (byte)0x00, - (byte)0x00, (byte)0x00, (byte)0x01, (byte)0x02, - (byte)0x06, (byte)0x00, (byte)0x02, (byte)0x00, - (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00, - }; - List records = testData(); - 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 final class SerializerVisitor implements RecordVisitor { + private final byte[] _buf; + private int _writeIndex; + public SerializerVisitor(byte[] buf) { + _buf = buf; + _writeIndex = 0; + + } + public void visitRecord(Record r) { + r.serialize(_writeIndex, _buf); + _writeIndex += r.getRecordSize(); + } + public int getWriteIndex() { + return _writeIndex; + } } - 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 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 (FormulaRecord.isSharedFormula()) is set when + * Sometimes the 'shared formula' flag (FormulaRecord.isSharedFormula()) 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).
- * So it is important that the code which locates the SharedFormulaRecord for each - * FormulaRecord does not allow matches across sheets.
- * - * Prior to bugzilla 44449 (Feb 2008), POI ValueRecordsAggregate.construct(int, List) + * So it is important that the code which locates the SharedFormulaRecord for each + * FormulaRecord does not allow matches across sheets.
+ * + * Prior to bugzilla 44449 (Feb 2008), POI ValueRecordsAggregate.construct(int, List) * allowed SharedFormulaRecords to be erroneously used across sheets. That incorrect * behaviour is shown by this test.

- * + * * Notes on how to produce the test spreadsheet:

- * 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.
*
    *
  1. A new spreadsheet was created in Excel (File | New | Blank Workbook).
    2. @@ -207,15 +219,15 @@ public final class TestValueRecordsAggregate extends TestCase { *
  2. Four rows on Sheet1 "5" through "8" were deleted ('delete rows' alt-E D, not 'clear' Del).
    3. *
  3. The spreadsheet was saved as AbnormalSharedFormulaFlag.xls.
    4. *
- * Prior to the row delete action the spreadsheet has two SharedFormulaRecords. One + * Prior to the row delete action the spreadsheet has two SharedFormulaRecords. One * for each sheet. To expose the bug, the shared formulas have been made to overlap.
- * The row delete action (as described here) seems to to delete the + * The row delete action (as described here) seems to to delete the * SharedFormulaRecord from Sheet1 (but not clear the 'shared formula' flags.
- * 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 - // peculiar condition we are testing for). + // It is easy to upset the test spreadsheet (for example re-saving will destroy the + // 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); + } }