From d5cbbaa1abcfb227f21afc5c9b1c1372d8a91716 Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Thu, 28 Aug 2008 04:27:41 +0000 Subject: [PATCH] Fix for bug 45699 - RowRecordsAggregate needs to tolerate MergeCellsRecords between row/cell records git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@689716 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/hssf/model/RecordOrderer.java | 38 +++++ src/java/org/apache/poi/hssf/model/Sheet.java | 99 +++++++------ .../record/aggregates/MergedCellsTable.java | 14 +- .../aggregates/RowRecordsAggregate.java | 56 ++++++- .../aggregates/SharedFormulaHolder.java | 96 ++++++++++++ .../aggregates/ValueRecordsAggregate.java | 138 +++++++++--------- .../org/apache/poi/hssf/model/TestSheet.java | 41 +++++- .../aggregates/TestValueRecordsAggregate.java | 52 +++---- 8 files changed, 378 insertions(+), 156 deletions(-) create mode 100644 src/java/org/apache/poi/hssf/record/aggregates/SharedFormulaHolder.java diff --git a/src/java/org/apache/poi/hssf/model/RecordOrderer.java b/src/java/org/apache/poi/hssf/model/RecordOrderer.java index e30e0a2a6e..291bd07715 100644 --- a/src/java/org/apache/poi/hssf/model/RecordOrderer.java +++ b/src/java/org/apache/poi/hssf/model/RecordOrderer.java @@ -22,16 +22,20 @@ import java.util.List; import org.apache.poi.hssf.record.BOFRecord; import org.apache.poi.hssf.record.CalcCountRecord; import org.apache.poi.hssf.record.CalcModeRecord; +import org.apache.poi.hssf.record.DVALRecord; import org.apache.poi.hssf.record.DateWindow1904Record; import org.apache.poi.hssf.record.DefaultRowHeightRecord; import org.apache.poi.hssf.record.DeltaRecord; import org.apache.poi.hssf.record.DimensionsRecord; +import org.apache.poi.hssf.record.DrawingRecord; +import org.apache.poi.hssf.record.DrawingSelectionRecord; import org.apache.poi.hssf.record.EOFRecord; import org.apache.poi.hssf.record.GridsetRecord; import org.apache.poi.hssf.record.GutsRecord; import org.apache.poi.hssf.record.HyperlinkRecord; import org.apache.poi.hssf.record.IndexRecord; import org.apache.poi.hssf.record.IterationRecord; +import org.apache.poi.hssf.record.ObjRecord; import org.apache.poi.hssf.record.PaneRecord; import org.apache.poi.hssf.record.PrecisionRecord; import org.apache.poi.hssf.record.PrintGridlinesRecord; @@ -42,8 +46,10 @@ import org.apache.poi.hssf.record.RefModeRecord; import org.apache.poi.hssf.record.SCLRecord; import org.apache.poi.hssf.record.SaveRecalcRecord; import org.apache.poi.hssf.record.SelectionRecord; +import org.apache.poi.hssf.record.TextObjectRecord; import org.apache.poi.hssf.record.UncalcedRecord; import org.apache.poi.hssf.record.UnknownRecord; +import org.apache.poi.hssf.record.WindowOneRecord; import org.apache.poi.hssf.record.WindowTwoRecord; import org.apache.poi.hssf.record.aggregates.ConditionalFormattingTable; import org.apache.poi.hssf.record.aggregates.DataValidityTable; @@ -161,12 +167,19 @@ final class RecordOrderer { private static int findInsertPosForNewMergedRecordTable(List records) { for (int i = records.size() - 2; i >= 0; i--) { // -2 to skip EOF record Object rb = records.get(i); + if (!(rb instanceof Record)) { + // DataValidityTable, ConditionalFormattingTable, + // even PageSettingsBlock (which doesn't normally appear after 'View Settings') + continue; + } Record rec = (Record) rb; switch (rec.getSid()) { + // 'View Settings' (4 records) case WindowTwoRecord.sid: case SCLRecord.sid: case PaneRecord.sid: case SelectionRecord.sid: + case UnknownRecord.STANDARDWIDTH_0099: return i + 1; } @@ -306,4 +319,29 @@ final class RecordOrderer { } return false; } + /** + * @return true if the specified record ID terminates a sequence of Row block records + * It is assumed that at least one row or cell value record has been found prior to the current + * record + */ + public static boolean isEndOfRowBlock(short sid) { + switch(sid) { + case DrawingRecord.sid: + case DrawingSelectionRecord.sid: + case ObjRecord.sid: + case TextObjectRecord.sid: + + case WindowOneRecord.sid: + // should really be part of workbook stream, but some apps seem to put this before WINDOW2 + case WindowTwoRecord.sid: + return true; + + case DVALRecord.sid: + return true; + case EOFRecord.sid: + // WINDOW2 should always be present, so shouldn't have got this far + throw new RuntimeException("Found EOFRecord before WindowTwoRecord was encountered"); + } + return PageSettingsBlock.isComponentRecord(sid); + } } diff --git a/src/java/org/apache/poi/hssf/model/Sheet.java b/src/java/org/apache/poi/hssf/model/Sheet.java index 336b1dbd46..b5db343d8e 100644 --- a/src/java/org/apache/poi/hssf/model/Sheet.java +++ b/src/java/org/apache/poi/hssf/model/Sheet.java @@ -122,7 +122,8 @@ public final class Sheet implements Model { protected WindowTwoRecord windowTwo = null; protected SelectionRecord selection = null; - private MergedCellsTable _mergedCellsTable; + /** java object always present, but if empty no BIFF records are written */ + private final MergedCellsTable _mergedCellsTable; /** always present in this POI object, not always written to Excel file */ /*package*/ColumnInfoRecordsAggregate _columnInfos; /** the DimensionsRecord is always present */ @@ -146,8 +147,8 @@ public final class Sheet implements Model { * Creates new Sheet with no initialization --useless at this point * @see #createSheet(List,int,int) */ - public Sheet() - { + public Sheet() { + _mergedCellsTable = new MergedCellsTable(); } /** @@ -158,7 +159,7 @@ public final class Sheet implements Model { * to the passed in records and references to those records held. This function * is normally called via Workbook. * - * @param recs array containing those records in the sheet in sequence (normally obtained from RecordFactory) + * @param inRecs array containing those records in the sheet in sequence (normally obtained from RecordFactory) * @param sheetnum integer specifying the sheet's number (0,1 or 2 in this release) * @param offset of the sheet's BOF record * @@ -167,19 +168,19 @@ public final class Sheet implements Model { * @see org.apache.poi.hssf.model.Workbook * @see org.apache.poi.hssf.record.Record */ - public static Sheet createSheet(List recs, int sheetnum, int offset) + public static Sheet createSheet(List inRecs, int sheetnum, int offset) { if (log.check( POILogger.DEBUG )) log.logFormatted(POILogger.DEBUG, "Sheet createSheet (existing file) with %", - new Integer(recs.size())); + new Integer(inRecs.size())); Sheet retval = new Sheet(); - ArrayList records = new ArrayList(recs.size() / 5); - boolean isfirstcell = true; - int bofEofNestingLevel = 0; + ArrayList records = new ArrayList(inRecs.size() / 5); + // TODO - take chart streams off into separate java objects + int bofEofNestingLevel = 0; // nesting level can only get to 2 (when charts are present) - for (int k = offset; k < recs.size(); k++) { - Record rec = ( Record ) recs.get(k); + for (int k = offset; k < inRecs.size(); k++) { + Record rec = ( Record ) inRecs.get(k); if ( rec.getSid() == DBCellRecord.sid ) { continue; } @@ -193,7 +194,7 @@ public final class Sheet implements Model { } if ( rec.getSid() == CFHeaderRecord.sid ) { - RecordStream rs = new RecordStream(recs, k); + RecordStream rs = new RecordStream(inRecs, k); retval.condFormatting = new ConditionalFormattingTable(rs); k += rs.getCountRead()-1; records.add(retval.condFormatting); @@ -201,43 +202,34 @@ public final class Sheet implements Model { } if (rec.getSid() == ColumnInfoRecord.sid) { - RecordStream rs = new RecordStream(recs, k); + RecordStream rs = new RecordStream(inRecs, k); retval._columnInfos = new ColumnInfoRecordsAggregate(rs); k += rs.getCountRead()-1; records.add(retval._columnInfos); continue; } if ( rec.getSid() == DVALRecord.sid) { - RecordStream rs = new RecordStream(recs, k); + RecordStream rs = new RecordStream(inRecs, k); retval._dataValidityTable = new DataValidityTable(rs); k += rs.getCountRead() - 1; // TODO - convert this method result to be zero based records.add(retval._dataValidityTable); continue; } // TODO construct RowRecordsAggregate from RecordStream - if ( rec.getSid() == RowRecord.sid ) { - RowRecord row = (RowRecord)rec; - if (retval._rowsAggregate == null) { - retval._rowsAggregate = new RowRecordsAggregate(); - records.add(retval._rowsAggregate); //only add the aggregate once + if ((rec.getSid() == RowRecord.sid || rec.isValue()) && bofEofNestingLevel == 1 ) { + //only add the aggregate once + if (retval._rowsAggregate != null) { + throw new RuntimeException("row/cell records found in the wrong place"); } - retval._rowsAggregate.insertRow(row); + int lastRowCellRec = findEndOfRowBlock(inRecs, k, retval._mergedCellsTable); + retval._rowsAggregate = new RowRecordsAggregate(inRecs, k, lastRowCellRec); + records.add(retval._rowsAggregate); //only add the aggregate once + k = lastRowCellRec -1; continue; } - if ( rec.isValue() && bofEofNestingLevel == 1 ) { - if (isfirstcell) { - isfirstcell = false; - if (retval._rowsAggregate == null) { - retval._rowsAggregate = new RowRecordsAggregate(); - records.add(retval._rowsAggregate); //only add the aggregate once - } - retval._rowsAggregate.constructCellValues( k, recs ); - } - continue; - } if (PageSettingsBlock.isComponentRecord(rec.getSid())) { - RecordStream rs = new RecordStream(recs, k); + RecordStream rs = new RecordStream(inRecs, k); PageSettingsBlock psb = new PageSettingsBlock(rs); if (bofEofNestingLevel == 1) { if (retval._psBlock == null) { @@ -253,9 +245,10 @@ public final class Sheet implements Model { } if (rec.getSid() == MergeCellsRecord.sid) { - RecordStream rs = new RecordStream(recs, k); - retval._mergedCellsTable = new MergedCellsTable(rs); - records.add(retval._mergedCellsTable); + // 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; } @@ -337,6 +330,11 @@ public final class Sheet implements Model { if (retval._dimensions == null) { throw new RuntimeException("DimensionsRecord was not found"); } + if (retval.windowTwo == null) { + throw new RuntimeException("WINDOW2 was not found"); + } + // put merged cells table in the right place (regardless of where the first MergedCellsRecord was found */ + RecordOrderer.addNewSheetRecord(records, retval._mergedCellsTable); retval.records = records; retval.checkRows(); if (log.check( POILogger.DEBUG )) @@ -344,6 +342,26 @@ 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; iRowRecordsAggregate + * + * @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 SharedFormulaRecords 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 SharedFormulaRecord.convertSharedFormulaRecord and hence the + * parsedExpression field of this FormulaRecord will not get updated.
+ * As it turns out, this is not a problem, because in these circumstances, the existing value + * for parsedExpression is perfectly OK.

+ * + * This method may also be used for setting breakpoints to help diagnose issues regarding the + * abnormally-set 'shared formula' flags. + * (see TestValueRecordsAggregate.testSpuriousSharedFormulaFlag()).

+ * + * 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. + } +} 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 8d5764187f..ae2fcf34e1 100644 --- a/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java +++ b/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java @@ -22,11 +22,14 @@ import java.util.Iterator; import java.util.List; import org.apache.poi.hssf.record.CellValueRecordInterface; -import org.apache.poi.hssf.record.EOFRecord; +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.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; @@ -39,8 +42,8 @@ import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor; * @author Jason Height (jheight at chariot dot net dot au) */ public final class ValueRecordsAggregate { - private int firstcell = -1; - private int lastcell = -1; + private int firstcell = -1; + private int lastcell = -1; private CellValueRecordInterface[][] records; /** Creates a new instance of ValueRecordsAggregate */ @@ -137,92 +140,81 @@ public final class ValueRecordsAggregate { return lastcell; } - public int construct(int offset, List records) - { + /** + * Processes a sequential group of cell value records. Stops at endIx or the first + * non-value record encountered. + * @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) { int k = 0; FormulaRecordAggregate lastFormulaAggregate = null; - // First up, locate all the shared formulas for this sheet - List sharedFormulas = new java.util.ArrayList(); - for (k = offset; k < records.size(); k++) - { + // Now do the main processing sweep + for (k = offset; k < endIx; k++) { Record rec = ( Record ) records.get(k); + + if (rec instanceof StringRecord) { + if (lastFormulaAggregate == null) { + throw new RuntimeException("StringRecord found without preceding FormulaRecord"); + } + if (lastFormulaAggregate.getStringRecord() != null) { + throw new RuntimeException("Multiple StringRecords found after FormulaRecord"); + } + lastFormulaAggregate.setStringRecord((StringRecord)rec); + lastFormulaAggregate = null; + continue; + } + + if (rec instanceof TableRecord) { + // TODO - don't loose this record + // DATATABLE probably belongs in formula record aggregate + if (lastFormulaAggregate == null) { + throw new RuntimeException("No preceding formula record found"); + } + lastFormulaAggregate = null; + continue; + } + if (rec instanceof SharedFormulaRecord) { - sharedFormulas.add(rec); + // Already handled, not to worry + continue; } - if(rec instanceof EOFRecord) { - // End of current sheet. Ignore all subsequent shared formula records (Bugzilla 44449) + + if (rec instanceof UnknownRecord) { break; } - } - - // Now do the main processing sweep - for (k = offset; k < records.size(); k++) - { - Record rec = ( Record ) records.get(k); - - if (rec instanceof StringRecord == false && !rec.isInValueSection() && !(rec instanceof UnknownRecord)) - { + if (rec instanceof RowRecord) { + break; + } + if (rec instanceof DBCellRecord) { + // end of 'Row Block'. This record is ignored by POI break; - } else if (rec instanceof SharedFormulaRecord) { - // Already handled, not to worry - } else if (rec instanceof FormulaRecord) - { - FormulaRecord formula = (FormulaRecord)rec; - if (formula.isSharedFormula()) { - // Traverse the list of shared formulas in - // reverse order, and try to find the correct one - // for us - boolean found = false; - for (int i=sharedFormulas.size()-1;i>=0;i--) { - // TODO - there is no junit test case to justify this reversed loop - // perhaps it could just run in the normal direction? - SharedFormulaRecord shrd = (SharedFormulaRecord)sharedFormulas.get(i); - if (shrd.isFormulaInShared(formula)) { - shrd.convertSharedFormulaRecord(formula); - found = true; - break; - } - } - if (!found) { - handleMissingSharedFormulaRecord(formula); - } - } - - lastFormulaAggregate = new FormulaRecordAggregate((FormulaRecord)rec, null); - insertCell( lastFormulaAggregate ); } - else if (rec instanceof StringRecord) - { - lastFormulaAggregate.setStringRecord((StringRecord)rec); + if (rec instanceof MergeCellsRecord) { + // 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"); } - else if (rec.isValue()) - { - insertCell(( CellValueRecordInterface ) rec); + if (rec instanceof FormulaRecord) { + FormulaRecord formula = (FormulaRecord)rec; + if (formula.isSharedFormula()) { + sfh.convertSharedFormulaRecord(formula); + } + + lastFormulaAggregate = new FormulaRecordAggregate((FormulaRecord)rec, null); + insertCell( lastFormulaAggregate ); + continue; } + insertCell(( CellValueRecordInterface ) rec); } - return k; + return k - offset - 1; } - /** - * Sometimes the shared formula flag "seems" to be erroneously set, in which case there is no - * call to SharedFormulaRecord.convertSharedFormulaRecord and hence the - * parsedExpression field of this FormulaRecord will not get updated.
- * As it turns out, this is not a problem, because in these circumstances, the existing value - * for parsedExpression is perfectly OK.

- * - * This method may also be used for setting breakpoints to help diagnose issues regarding the - * abnormally-set 'shared formula' flags. - * (see TestValueRecordsAggregate.testSpuriousSharedFormulaFlag()).

- * - * 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. - } - /** Tallies a count of the size of the cell records * that are attached to the rows in the range specified. */ diff --git a/src/testcases/org/apache/poi/hssf/model/TestSheet.java b/src/testcases/org/apache/poi/hssf/model/TestSheet.java index 6c6dd9fb25..97635e5362 100644 --- a/src/testcases/org/apache/poi/hssf/model/TestSheet.java +++ b/src/testcases/org/apache/poi/hssf/model/TestSheet.java @@ -24,6 +24,7 @@ import java.util.List; 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; @@ -32,6 +33,7 @@ import org.apache.poi.hssf.record.CellValueRecordInterface; import org.apache.poi.hssf.record.ColumnInfoRecord; import org.apache.poi.hssf.record.DimensionsRecord; import org.apache.poi.hssf.record.EOFRecord; +import org.apache.poi.hssf.record.FormulaRecord; import org.apache.poi.hssf.record.GutsRecord; import org.apache.poi.hssf.record.IndexRecord; import org.apache.poi.hssf.record.MergeCellsRecord; @@ -39,9 +41,13 @@ import org.apache.poi.hssf.record.Record; import org.apache.poi.hssf.record.RowRecord; import org.apache.poi.hssf.record.StringRecord; import org.apache.poi.hssf.record.UncalcedRecord; +import org.apache.poi.hssf.record.WindowTwoRecord; 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.usermodel.HSSFCell; +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.hssf.util.CellRangeAddress; @@ -57,6 +63,7 @@ public final class TestSheet extends TestCase { List records = new ArrayList(); records.add( new BOFRecord() ); records.add( new DimensionsRecord() ); + records.add(createWindow2Record()); records.add(EOFRecord.instance); Sheet sheet = Sheet.createSheet( records, 0, 0 ); @@ -65,9 +72,22 @@ public final class TestSheet extends TestCase { assertTrue( sheet.records.get(pos++) instanceof ColumnInfoRecordsAggregate ); assertTrue( sheet.records.get(pos++) instanceof DimensionsRecord ); assertTrue( sheet.records.get(pos++) instanceof RowRecordsAggregate ); + assertTrue( sheet.records.get(pos++) instanceof WindowTwoRecord ); + assertTrue( sheet.records.get(pos++) instanceof MergedCellsTable ); assertTrue( sheet.records.get(pos++) instanceof EOFRecord ); } + private static Record createWindow2Record() { + WindowTwoRecord result = new WindowTwoRecord(); + result.setOptions(( short ) 0x6b6); + result.setTopRow(( short ) 0); + result.setLeftCol(( short ) 0); + result.setHeaderColor(0x40); + result.setPageBreakZoom(( short ) 0); + result.setNormalZoom(( short ) 0); + return result; + } + private static final class MergedCellListener implements ERFListener { private int _count; @@ -168,6 +188,8 @@ public final class TestSheet extends TestCase { records.add(new RowRecord(0)); records.add(new RowRecord(1)); records.add(new RowRecord(2)); + records.add(createWindow2Record()); + records.add(EOFRecord.instance); records.add(merged); Sheet sheet = Sheet.createSheet(records, 0); @@ -193,11 +215,15 @@ public final class TestSheet extends TestCase { public void testRowAggregation() { List records = new ArrayList(); + records.add(Sheet.createBOF()); records.add(new DimensionsRecord()); records.add(new RowRecord(0)); records.add(new RowRecord(1)); + records.add(new FormulaRecord()); records.add(new StringRecord()); records.add(new RowRecord(2)); + records.add(createWindow2Record()); + records.add(EOFRecord.instance); Sheet sheet = Sheet.createSheet(records, 0); assertNotNull("Row [2] was skipped", sheet.getRow(2)); @@ -400,6 +426,7 @@ public final class TestSheet extends TestCase { records.add(new BOFRecord()); records.add(new UncalcedRecord()); records.add(new DimensionsRecord()); + records.add(createWindow2Record()); records.add(EOFRecord.instance); Sheet sheet = Sheet.createSheet(records, 0, 0); @@ -408,7 +435,7 @@ public final class TestSheet extends TestCase { if (serializedSize != estimatedSize) { throw new AssertionFailedError("Identified bug 45066 b"); } - assertEquals(68, serializedSize); + assertEquals(90, serializedSize); } /** @@ -502,5 +529,17 @@ public final class TestSheet extends TestCase { } assertEquals(1, count); } + + public void testMisplacedMergedCellsRecords_bug45699() { + HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex45698-22488.xls"); + + HSSFSheet sheet = wb.getSheetAt(0); + HSSFRow row = sheet.getRow(3); + HSSFCell cell = row.getCell(4); + if (cell == null) { + throw new AssertionFailedError("Identified bug 45699"); + } + assertEquals("Informations", cell.getRichStringCellValue().getString()); + } } 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 d85b50a20c..0904fe1516 100755 --- a/src/testcases/org/apache/poi/hssf/record/aggregates/TestValueRecordsAggregate.java +++ b/src/testcases/org/apache/poi/hssf/record/aggregates/TestValueRecordsAggregate.java @@ -17,8 +17,6 @@ package org.apache.poi.hssf.record.aggregates; -import java.io.File; -import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; @@ -34,27 +32,23 @@ import org.apache.poi.hssf.record.BlankRecord; import org.apache.poi.hssf.record.FormulaRecord; import org.apache.poi.hssf.record.Record; import org.apache.poi.hssf.record.SharedFormulaRecord; -import org.apache.poi.hssf.record.UnknownRecord; -import org.apache.poi.hssf.record.WindowOneRecord; import org.apache.poi.hssf.usermodel.HSSFSheet; import org.apache.poi.hssf.usermodel.HSSFWorkbook; -public class TestValueRecordsAggregate extends TestCase -{ +public final class TestValueRecordsAggregate extends TestCase { private static final String ABNORMAL_SHARED_FORMULA_FLAG_TEST_FILE = "AbnormalSharedFormulaFlag.xls"; - ValueRecordsAggregate valueRecord = new ValueRecordsAggregate(); + private final ValueRecordsAggregate valueRecord = new ValueRecordsAggregate(); /** * Make sure the shared formula DOESNT makes it to the FormulaRecordAggregate when being parsed * as part of the value records */ - public void testSharedFormula() - { + public void testSharedFormula() { List records = new ArrayList(); records.add( new FormulaRecord() ); records.add( new SharedFormulaRecord() ); - valueRecord.construct( 0, records ); + constructValueRecord(records); Iterator iterator = valueRecord.getIterator(); Record record = (Record) iterator.next(); assertNotNull( "Row contains a value", record ); @@ -64,24 +58,25 @@ public class TestValueRecordsAggregate extends TestCase } - private List testData(){ + private void constructValueRecord(List records) { + SharedFormulaHolder sfrh = SharedFormulaHolder.create(records, 0, records.size()); + valueRecord.construct(records, 0, records.size(), sfrh ); + } + + private static List testData() { List records = new ArrayList(); FormulaRecord formulaRecord = new FormulaRecord(); BlankRecord blankRecord = new BlankRecord(); - WindowOneRecord windowOneRecord = new WindowOneRecord(); formulaRecord.setRow( 1 ); formulaRecord.setColumn( (short) 1 ); blankRecord.setRow( 2 ); blankRecord.setColumn( (short) 2 ); records.add( formulaRecord ); records.add( blankRecord ); - records.add( windowOneRecord ); return records; } - public void testInsertCell() - throws Exception - { + public void testInsertCell() { Iterator iterator = valueRecord.getIterator(); assertFalse( iterator.hasNext() ); @@ -103,8 +98,7 @@ public class TestValueRecordsAggregate extends TestCase valueRecord.removeCell( blankRecord2 ); } - public void testGetPhysicalNumberOfCells() throws Exception - { + public void testGetPhysicalNumberOfCells() { assertEquals(0, valueRecord.getPhysicalNumberOfCells()); BlankRecord blankRecord1 = newBlankRecord(); valueRecord.insertCell( blankRecord1 ); @@ -113,8 +107,7 @@ public class TestValueRecordsAggregate extends TestCase assertEquals(0, valueRecord.getPhysicalNumberOfCells()); } - public void testGetFirstCellNum() throws Exception - { + public void testGetFirstCellNum() { assertEquals( -1, valueRecord.getFirstCellNum() ); valueRecord.insertCell( newBlankRecord( 2, 2 ) ); assertEquals( 2, valueRecord.getFirstCellNum() ); @@ -126,8 +119,7 @@ public class TestValueRecordsAggregate extends TestCase assertEquals( 2, valueRecord.getFirstCellNum() ); } - public void testGetLastCellNum() throws Exception - { + public void testGetLastCellNum() { assertEquals( -1, valueRecord.getLastCellNum() ); valueRecord.insertCell( newBlankRecord( 2, 2 ) ); assertEquals( 2, valueRecord.getLastCellNum() ); @@ -140,8 +132,7 @@ public class TestValueRecordsAggregate extends TestCase } - public void testSerialize() throws Exception - { + public void testSerialize() { byte[] actualArray = new byte[36]; byte[] expectedArray = new byte[] { @@ -156,7 +147,7 @@ public class TestValueRecordsAggregate extends TestCase (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x00, }; List records = testData(); - valueRecord.construct( 0, records ); + constructValueRecord(records); int bytesWritten = valueRecord.serializeCellRow(1, 0, actualArray ); bytesWritten += valueRecord.serializeCellRow(2, bytesWritten, actualArray ); assertEquals( 36, bytesWritten ); @@ -164,18 +155,12 @@ public class TestValueRecordsAggregate extends TestCase assertEquals( expectedArray[i], actualArray[i] ); } - public static void main( String[] args ) - { - System.out.println( "Testing org.apache.poi.hssf.record.aggregates.TestValueRecordAggregate" ); - junit.textui.TestRunner.run( TestValueRecordsAggregate.class ); - } - - private BlankRecord newBlankRecord() + private static BlankRecord newBlankRecord() { return newBlankRecord( 2, 2 ); } - private BlankRecord newBlankRecord( int col, int row) + private static BlankRecord newBlankRecord( int col, int row) { BlankRecord blankRecord = new BlankRecord(); blankRecord.setRow( row ); @@ -285,5 +270,4 @@ public class TestValueRecordsAggregate extends TestCase return crc.getValue(); } - } -- 2.39.5