Fix for bug 46280 - RowRecordsAggregate should allow for ContinueRecords after UnkownRecords, and PivotTable records should not get in the RowRecordsAggregate at all

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@720318 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Josh Micich 2008-11-24 22:40:46 +00:00
parent 1559edee21
commit 7735bb1b3b
9 changed files with 529 additions and 440 deletions

View File

@ -37,7 +37,7 @@
<!-- Don't forget to update status.xml too! -->
<release version="3.5-beta5" date="2008-??-??">
<action dev="POI-DEVELOPERS" type="fix"><!--remove me--></action>
<action dev="POI-DEVELOPERS" type="fix">46280 - Fixed RowRecordsAggregate etc to properly skip PivotTable records</action>
</release>
<release version="3.5-beta4" date="2008-11-29">
<action dev="POI-DEVELOPERS" type="fix">46213 - Fixed FormulaRecordAggregate to gracefully ignore extra StringRecords</action>

View File

@ -34,7 +34,7 @@
<!-- Don't forget to update changes.xml too! -->
<changes>
<release version="3.5-beta5" date="2008-??-??">
<action dev="POI-DEVELOPERS" type="fix"><!--remove me--></action>
<action dev="POI-DEVELOPERS" type="fix">46280 - Fixed RowRecordsAggregate etc to properly skip PivotTable records</action>
</release>
<release version="3.5-beta4" date="2008-11-29">
<action dev="POI-DEVELOPERS" type="fix">46213 - Fixed FormulaRecordAggregate to gracefully ignore extra StringRecords</action>

View File

@ -85,12 +85,12 @@ final class RecordOrderer {
* Adds the specified new record in the correct place in sheet records list
*
*/
public static void addNewSheetRecord(List sheetRecords, RecordBase newRecord) {
public static void addNewSheetRecord(List<RecordBase> sheetRecords, RecordBase newRecord) {
int index = findSheetInsertPos(sheetRecords, newRecord.getClass());
sheetRecords.add(index, newRecord);
}
private static int findSheetInsertPos(List records, Class recClass) {
private static int findSheetInsertPos(List<RecordBase> records, Class recClass) {
if (recClass == DataValidityTable.class) {
return findDataValidationTableInsertPos(records);
}
@ -109,7 +109,7 @@ final class RecordOrderer {
throw new RuntimeException("Unexpected record class (" + recClass.getName() + ")");
}
private static int getPageBreakRecordInsertPos(List records) {
private static int getPageBreakRecordInsertPos(List<RecordBase> records) {
int dimensionsIndex = getDimensionsIndex(records);
int i = dimensionsIndex-1;
while (i > 0) {
@ -152,7 +152,7 @@ final class RecordOrderer {
/**
* Find correct position to add new CFHeader record
*/
private static int findInsertPosForNewCondFormatTable(List records) {
private static int findInsertPosForNewCondFormatTable(List<RecordBase> records) {
for (int i = records.size() - 2; i >= 0; i--) { // -2 to skip EOF record
Object rb = records.get(i);
@ -175,7 +175,7 @@ final class RecordOrderer {
throw new RuntimeException("Did not find Window2 record");
}
private static int findInsertPosForNewMergedRecordTable(List records) {
private static int findInsertPosForNewMergedRecordTable(List<RecordBase> records) {
for (int i = records.size() - 2; i >= 0; i--) { // -2 to skip EOF record
Object rb = records.get(i);
if (!(rb instanceof Record)) {
@ -219,14 +219,14 @@ final class RecordOrderer {
* o RANGEPROTECTION
* + EOF
*/
private static int findDataValidationTableInsertPos(List records) {
private static int findDataValidationTableInsertPos(List<RecordBase> records) {
int i = records.size() - 1;
if (!(records.get(i) instanceof EOFRecord)) {
throw new IllegalStateException("Last sheet record should be EOFRecord");
}
while (i > 0) {
i--;
Object rb = records.get(i);
RecordBase rb = records.get(i);
if (isDVTPriorRecord(rb)) {
Record nextRec = (Record) records.get(i + 1);
if (!isDVTSubsequentRecord(nextRec.getSid())) {
@ -245,7 +245,7 @@ final class RecordOrderer {
}
private static boolean isDVTPriorRecord(Object rb) {
private static boolean isDVTPriorRecord(RecordBase rb) {
if (rb instanceof MergedCellsTable || rb instanceof ConditionalFormattingTable) {
return true;
}
@ -280,7 +280,7 @@ final class RecordOrderer {
/**
* DIMENSIONS record is always present
*/
private static int getDimensionsIndex(List records) {
private static int getDimensionsIndex(List<RecordBase> records) {
int nRecs = records.size();
for(int i=0; i<nRecs; i++) {
if(records.get(i) instanceof DimensionsRecord) {
@ -291,12 +291,12 @@ final class RecordOrderer {
throw new RuntimeException("DimensionsRecord not found");
}
private static int getGutsRecordInsertPos(List records) {
private static int getGutsRecordInsertPos(List<RecordBase> records) {
int dimensionsIndex = getDimensionsIndex(records);
int i = dimensionsIndex-1;
while (i > 0) {
i--;
Object rb = records.get(i);
RecordBase rb = records.get(i);
if (isGutsPriorRecord(rb)) {
return i+1;
}
@ -304,7 +304,7 @@ final class RecordOrderer {
throw new RuntimeException("Did not find insert point for GUTS");
}
private static boolean isGutsPriorRecord(Object rb) {
private static boolean isGutsPriorRecord(RecordBase rb) {
if (rb instanceof Record) {
Record record = (Record) rb;
switch (record.getSid()) {
@ -337,6 +337,8 @@ final class RecordOrderer {
*/
public static boolean isEndOfRowBlock(int sid) {
switch(sid) {
case UnknownRecord.SXVIEW_00B0:
// should have been prefixed with DrawingRecord (0x00EC), but bug 46280 seems to allow this
case DrawingRecord.sid:
case DrawingSelectionRecord.sid:
case ObjRecord.sid:

View File

@ -45,11 +45,11 @@ public final class RowBlocksReader {
* mergedCellsTable
*/
public RowBlocksReader(RecordStream rs) {
List plainRecords = new ArrayList();
List shFrmRecords = new ArrayList();
List arrayRecords = new ArrayList();
List tableRecords = new ArrayList();
List mergeCellRecords = new ArrayList();
List<Record> plainRecords = new ArrayList<Record>();
List<Record> shFrmRecords = new ArrayList<Record>();
List<Record> arrayRecords = new ArrayList<Record>();
List<Record> tableRecords = new ArrayList<Record>();
List<Record> mergeCellRecords = new ArrayList<Record>();
while(!RecordOrderer.isEndOfRowBlock(rs.peekNextSid())) {
// End of row/cell records for the current sheet
@ -61,7 +61,7 @@ public final class RowBlocksReader {
}
Record rec = rs.getNext();
List dest;
List<Record> dest;
switch (rec.getSid()) {
case MergeCellsRecord.sid: dest = mergeCellRecords; break;
case SharedFormulaRecord.sid: dest = shFrmRecords; break;

View File

@ -39,6 +39,7 @@ public final class UnknownRecord extends StandardRecord {
public static final int SHEETPR_0081 = 0x0081;
public static final int STANDARDWIDTH_0099 = 0x0099;
public static final int SCL_00A0 = 0x00A0;
public static final int SXVIEW_00B0 = 0x00B0;
public static final int BITMAP_00E9 = 0x00E9;
public static final int PHONETICPR_00EF = 0x00EF;
public static final int LABELRANGES_015F = 0x015F;
@ -130,11 +131,20 @@ public final class UnknownRecord extends StandardRecord {
case 0x009D: return "AUTOFILTERINFO";
case SCL_00A0: return "SCL";
case 0x00AE: return "SCENMAN";
case SXVIEW_00B0: return "SXVIEW"; // (pivot table) View Definition
case 0x00B1: return "SXVD"; // (pivot table) View Fields
case 0x00B2: return "SXVI"; // (pivot table) View Item
case 0x00B4: return "SXIVD"; // (pivot table) Row/Column Field IDs
case 0x00B5: return "SXLI"; // (pivot table) Line Item Array
case 0x00C5: return "SXDI"; // (pivot table) Data Item
case 0x00D3: return "OBPROJ";
case 0x00DC: return "PARAMQRY";
case 0x00DE: return "OLESIZE";
case BITMAP_00E9: return "BITMAP";
case PHONETICPR_00EF: return "PHONETICPR";
case 0x00F1: return "SXEX"; // PivotTable View Extended Information
case 0x0100: return "SXVDEX"; // Extended PivotTable View Fields
case LABELRANGES_015F: return "LABELRANGES";
case 0x01BA: return "CODENAME";
@ -145,14 +155,16 @@ public final class UnknownRecord extends StandardRecord {
case 0x01C0: return "EXCEL9FILE";
case 0x0802: return "QSISXTAG";
case 0x0802: return "QSISXTAG"; // Pivot Table and Query Table Extensions
case 0x0803: return "DBQUERYEXT";
case 0x0805: return "TXTQUERY";
case 0x0810: return "SXVIEWEX9"; // Pivot Table Extensions
case 0x0812: return "CONTINUEFRT";
case QUICKTIP_0800: return "QUICKTIP";
case SHEETEXT_0862: return "SHEETEXT";
case 0x0863: return "BOOKEXT";
case 0x0864: return "SXADDL"; // Pivot Table Additional Info
case SHEETPROTECTION_0867: return "SHEETPROTECTION";
case RANGEPROTECTION_0868: return "RANGEPROTECTION";
case 0x086B: return "DATALABEXTCONTENTS";

View File

@ -26,6 +26,7 @@ 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.ContinueRecord;
import org.apache.poi.hssf.record.DBCellRecord;
import org.apache.poi.hssf.record.FormulaRecord;
import org.apache.poi.hssf.record.IndexRecord;
@ -45,9 +46,9 @@ import org.apache.poi.hssf.record.formula.FormulaShifter;
public final class RowRecordsAggregate extends RecordAggregate {
private int _firstrow = -1;
private int _lastrow = -1;
private final Map _rowRecords;
private final Map<Integer, RowRecord> _rowRecords;
private final ValueRecordsAggregate _valuesAgg;
private final List _unknownRecords;
private final List<Record> _unknownRecords;
private final SharedValueManager _sharedValueManager;
/** Creates a new instance of ValueRecordsAggregate */
@ -55,9 +56,9 @@ public final class RowRecordsAggregate extends RecordAggregate {
this(SharedValueManager.EMPTY);
}
private RowRecordsAggregate(SharedValueManager svm) {
_rowRecords = new TreeMap();
_rowRecords = new TreeMap<Integer, RowRecord>();
_valuesAgg = new ValueRecordsAggregate();
_unknownRecords = new ArrayList();
_unknownRecords = new ArrayList<Record>();
_sharedValueManager = svm;
}
@ -79,8 +80,11 @@ public final class RowRecordsAggregate extends RecordAggregate {
continue;
}
if (rec instanceof UnknownRecord) {
addUnknownRecord((UnknownRecord)rec);
// might need to keep track of where exactly these belong
addUnknownRecord(rec);
while (rs.peekNextSid() == ContinueRecord.sid) {
addUnknownRecord(rs.getNext());
}
continue;
}
if (!(rec instanceof CellValueRecordInterface)) {
@ -92,7 +96,7 @@ public final class RowRecordsAggregate extends RecordAggregate {
/**
* Handles UnknownRecords which appear within the row/cell records
*/
private void addUnknownRecord(UnknownRecord rec) {
private void addUnknownRecord(Record rec) {
// ony a few distinct record IDs are encountered by the existing POI test cases:
// 0x1065 // many
// 0x01C2 // several
@ -105,12 +109,10 @@ public final class RowRecordsAggregate extends RecordAggregate {
public void insertRow(RowRecord row) {
// Integer integer = new Integer(row.getRowNumber());
_rowRecords.put(new Integer(row.getRowNumber()), row);
if ((row.getRowNumber() < _firstrow) || (_firstrow == -1))
{
if ((row.getRowNumber() < _firstrow) || (_firstrow == -1)) {
_firstrow = row.getRowNumber();
}
if ((row.getRowNumber() > _lastrow) || (_lastrow == -1))
{
if ((row.getRowNumber() > _lastrow) || (_lastrow == -1)) {
_lastrow = row.getRowNumber();
}
}
@ -119,7 +121,7 @@ public final class RowRecordsAggregate extends RecordAggregate {
int rowIndex = row.getRowNumber();
_valuesAgg.removeAllCellsValuesForRow(rowIndex);
Integer key = new Integer(rowIndex);
RowRecord rr = (RowRecord) _rowRecords.remove(key);
RowRecord rr = _rowRecords.remove(key);
if (rr == null) {
throw new RuntimeException("Invalid row index (" + key.intValue() + ")");
}
@ -133,7 +135,7 @@ public final class RowRecordsAggregate extends RecordAggregate {
if (rowIndex < 0 || rowIndex > 65535) {
throw new IllegalArgumentException("The row number must be between 0 and 65535");
}
return (RowRecord) _rowRecords.get(new Integer(rowIndex));
return _rowRecords.get(new Integer(rowIndex));
}
public int getPhysicalNumberOfRows()
@ -270,7 +272,7 @@ public final class RowRecordsAggregate extends RecordAggregate {
}
for (int i=0; i< _unknownRecords.size(); i++) {
// Potentially breaking the file here since we don't know exactly where to write these records
rv.visitRecord((Record) _unknownRecords.get(i));
rv.visitRecord(_unknownRecords.get(i));
}
}
@ -278,41 +280,27 @@ public final class RowRecordsAggregate extends RecordAggregate {
return _rowRecords.values().iterator();
}
public Iterator getAllRecordsIterator() {
List result = new ArrayList(_rowRecords.size() * 2);
result.addAll(_rowRecords.values());
Iterator vi = _valuesAgg.getIterator();
while (vi.hasNext()) {
result.add(vi.next());
}
return result.iterator();
}
public int findStartOfRowOutlineGroup(int row)
{
public int findStartOfRowOutlineGroup(int row) {
// Find the start of the group.
RowRecord rowRecord = this.getRow( row );
int level = rowRecord.getOutlineLevel();
int currentRow = row;
while (this.getRow( currentRow ) != null)
{
while (this.getRow( currentRow ) != null) {
rowRecord = this.getRow( currentRow );
if (rowRecord.getOutlineLevel() < level)
if (rowRecord.getOutlineLevel() < level) {
return currentRow + 1;
}
currentRow--;
}
return currentRow + 1;
}
public int findEndOfRowOutlineGroup( int row )
{
public int findEndOfRowOutlineGroup(int row) {
int level = getRow( row ).getOutlineLevel();
int currentRow;
for (currentRow = row; currentRow < this.getLastRowNum(); currentRow++)
{
if (getRow(currentRow) == null || getRow(currentRow).getOutlineLevel() < level)
{
for (currentRow = row; currentRow < getLastRowNum(); currentRow++) {
if (getRow(currentRow) == null || getRow(currentRow).getOutlineLevel() < level) {
break;
}
}
@ -365,25 +353,24 @@ public final class RowRecordsAggregate extends RecordAggregate {
return new RowRecord(rowNumber);
}
public boolean isRowGroupCollapsed( int row )
{
public boolean isRowGroupCollapsed(int row) {
int collapseRow = findEndOfRowOutlineGroup(row) + 1;
if (getRow(collapseRow) == null)
if (getRow(collapseRow) == null) {
return false;
else
}
return getRow( collapseRow ).getColapsed();
}
public void expandRow( int rowNumber )
{
public void expandRow(int rowNumber) {
int idx = rowNumber;
if (idx == -1)
return;
// If it is already expanded do nothing.
if (!isRowGroupCollapsed(idx))
if (!isRowGroupCollapsed(idx)) {
return;
}
// Find the start of the group.
int startIdx = findStartOfRowOutlineGroup(idx);
@ -399,14 +386,12 @@ public final class RowRecordsAggregate extends RecordAggregate {
// to look at the start and the end of the current group to determine which
// is the enclosing group
// hidden bit only is altered for this outline level. ie. don't un-collapse contained groups
if ( !isRowGroupHiddenByParent( idx ) )
{
for ( int i = startIdx; i <= endIdx; i++ )
{
if ( row.getOutlineLevel() == getRow( i ).getOutlineLevel() )
getRow( i ).setZeroHeight( false );
else if (!isRowGroupCollapsed(i))
getRow( i ).setZeroHeight( false );
if (!isRowGroupHiddenByParent(idx)) {
for (int i = startIdx; i <= endIdx; i++) {
RowRecord otherRow = getRow(i);
if (row.getOutlineLevel() == otherRow.getOutlineLevel() || !isRowGroupCollapsed(i)) {
otherRow.setZeroHeight(false);
}
}
}
@ -414,19 +399,15 @@ public final class RowRecordsAggregate extends RecordAggregate {
getRow(endIdx + 1).setColapsed(false);
}
public boolean isRowGroupHiddenByParent( int row )
{
public boolean isRowGroupHiddenByParent(int row) {
// Look out outline details of end
int endLevel;
boolean endHidden;
int endOfOutlineGroupIdx = findEndOfRowOutlineGroup(row);
if (getRow( endOfOutlineGroupIdx + 1 ) == null)
{
if (getRow(endOfOutlineGroupIdx + 1) == null) {
endLevel = 0;
endHidden = false;
}
else
{
} else {
endLevel = getRow(endOfOutlineGroupIdx + 1).getOutlineLevel();
endHidden = getRow(endOfOutlineGroupIdx + 1).getZeroHeight();
}
@ -435,26 +416,20 @@ public final class RowRecordsAggregate extends RecordAggregate {
int startLevel;
boolean startHidden;
int startOfOutlineGroupIdx = findStartOfRowOutlineGroup( row );
if (startOfOutlineGroupIdx - 1 < 0 || getRow(startOfOutlineGroupIdx - 1) == null)
{
if (startOfOutlineGroupIdx - 1 < 0 || getRow(startOfOutlineGroupIdx - 1) == null) {
startLevel = 0;
startHidden = false;
}
else
{
} else {
startLevel = getRow(startOfOutlineGroupIdx - 1).getOutlineLevel();
startHidden = getRow(startOfOutlineGroupIdx - 1).getZeroHeight();
}
if (endLevel > startLevel)
{
if (endLevel > startLevel) {
return endHidden;
}
else
{
return startHidden;
}
}
public CellValueRecordInterface[] getValueRecords() {
return _valuesAgg.getValueRecords();

View File

@ -35,6 +35,7 @@ public final class AllModelTests {
result.addTestSuite(TestFormulaParserEval.class);
result.addTestSuite(TestFormulaParserIf.class);
result.addTestSuite(TestOperandClassTransformer.class);
result.addTestSuite(TestRowBlocksReader.class);
result.addTestSuite(TestRVA.class);
result.addTestSuite(TestSheet.class);
result.addTestSuite(TestSheetAdditional.class);

View File

@ -0,0 +1,60 @@
/* ====================================================================
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.Arrays;
import junit.framework.AssertionFailedError;
import junit.framework.TestCase;
import org.apache.poi.hssf.record.NumberRecord;
import org.apache.poi.hssf.record.Record;
import org.apache.poi.hssf.record.RowRecord;
import org.apache.poi.hssf.record.UnknownRecord;
import org.apache.poi.hssf.record.WindowTwoRecord;
/**
* Tests for {@link RowBlocksReader}
*
* @author Josh Micich
*/
public final class TestRowBlocksReader extends TestCase {
public void testAbnormalPivotTableRecords_bug46280() {
int SXVIEW_SID = 0x00B0;
Record[] inRecs = {
new RowRecord(0),
new NumberRecord(),
// normally MSODRAWING(0x00EC) would come here before SXVIEW
new UnknownRecord(SXVIEW_SID, "dummydata (SXVIEW: View Definition)".getBytes()),
new WindowTwoRecord(),
};
RecordStream rs = new RecordStream(Arrays.asList(inRecs), 0);
RowBlocksReader rbr = new RowBlocksReader(rs);
if (rs.peekNextClass() == WindowTwoRecord.class) {
// Should have stopped at the SXVIEW record
throw new AssertionFailedError("Identified bug 46280b");
}
RecordStream rbStream = rbr.getPlainRecordStream();
assertEquals(inRecs[0], rbStream.getNext());
assertEquals(inRecs[1], rbStream.getNext());
assertFalse(rbStream.hasNext());
assertTrue(rs.hasNext());
assertEquals(inRecs[2], rs.getNext());
assertEquals(inRecs[3], rs.getNext());
}
}

View File

@ -21,24 +21,30 @@ import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.util.Arrays;
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.record.ArrayRecord;
import org.apache.poi.hssf.record.ContinueRecord;
import org.apache.poi.hssf.record.FormulaRecord;
import org.apache.poi.hssf.record.NumberRecord;
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.record.UnknownRecord;
import org.apache.poi.hssf.usermodel.HSSFWorkbook;
import org.apache.poi.hssf.usermodel.RecordInspector;
import org.apache.poi.hssf.usermodel.RecordInspector.RecordCollector;
import org.apache.poi.hssf.util.CellRangeAddress8Bit;
/**
*
* Tests for {@link RowRecordsAggregate}
*/
public final class TestRowRecordsAggregate extends TestCase {
@ -113,4 +119,37 @@ public final class TestRowRecordsAggregate extends TestCase {
assertEquals(range.getFirstRow(), firstFormula.getRow());
assertEquals(range.getFirstColumn(), firstFormula.getColumn());
}
/**
* This problem was noted as the overt symptom of bug 46280. The logic for skipping {@link
* UnknownRecord}s in the constructor {@link RowRecordsAggregate} did not allow for the
* possibility of tailing {@link ContinueRecord}s.<br/>
* The functionality change being tested here is actually not critical to the overall fix
* for bug 46280, since the fix involved making sure the that offending <i>PivotTable</i>
* records do not get into {@link RowRecordsAggregate}.<br/>
* This fix in {@link RowRecordsAggregate} was implemented anyway since any {@link
* UnknownRecord} has the potential of being 'continued'.
*/
public void testUnknownContinue_bug46280() {
Record[] inRecs = {
new RowRecord(0),
new NumberRecord(),
new UnknownRecord(0x5555, "dummydata".getBytes()),
new ContinueRecord("moredummydata".getBytes()),
};
RecordStream rs = new RecordStream(Arrays.asList(inRecs), 0);
RowRecordsAggregate rra;
try {
rra = new RowRecordsAggregate(rs, SharedValueManager.EMPTY);
} catch (RuntimeException e) {
if (e.getMessage().startsWith("Unexpected record type")) {
throw new AssertionFailedError("Identified bug 46280a");
}
throw e;
}
RecordCollector rv = new RecordCollector();
rra.visitContainedRecords(rv);
Record[] outRecs = rv.getRecords();
assertEquals(5, outRecs.length);
}
}