From 2014a596be190c3257501f2c6698ce59894c891a Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Mon, 1 Dec 2008 19:59:46 +0000 Subject: [PATCH] Fix for bug 46312 - ValueRecordsAggregate should handle removal of new empty row git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@722206 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/changes.xml | 1 + src/documentation/content/xdocs/status.xml | 1 + .../aggregates/ValueRecordsAggregate.java | 508 +++++++++--------- .../aggregates/TestValueRecordsAggregate.java | 473 ++++++++-------- 4 files changed, 515 insertions(+), 468 deletions(-) diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 991bd0ec1..80a6e8cee 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + 46312 - Fixed ValueRecordsAggregate to handle removal of new empty row 46269 - Improved error message when attempting to read BIFF2 file 46206 - Fixed Sheet to tolerate missing DIMENSION records 46301 - added pivot table records: SXDI, SXVDEX, SXPI, SXIDSTM, SXVIEW, SXVD, SXVS, et al diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 1a1fd30eb..8763adc0a 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 46312 - Fixed ValueRecordsAggregate to handle removal of new empty row 46269 - Improved error message when attempting to read BIFF2 file 46206 - Fixed Sheet to tolerate missing DIMENSION records 46301 - added pivot table records: SXDI, SXVDEX, SXPI, SXIDSTM, SXVIEW, SXVD, SXVS, et al 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 27000811c..6ee871a7f 100644 --- a/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java +++ b/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java @@ -40,279 +40,291 @@ import org.apache.poi.hssf.record.formula.Ptg; * @author Jason Height (jheight at chariot dot net dot au) */ public final class ValueRecordsAggregate { - private int firstcell = -1; - private int lastcell = -1; - private CellValueRecordInterface[][] records; + private static final int MAX_ROW_INDEX = 0XFFFF; + private int firstcell = -1; + private int lastcell = -1; + private CellValueRecordInterface[][] records; - /** Creates a new instance of ValueRecordsAggregate */ + /** Creates a new instance of ValueRecordsAggregate */ - public ValueRecordsAggregate() - { - records = new CellValueRecordInterface[30][]; // We start with 30 Rows. - } + public ValueRecordsAggregate() { + records = new CellValueRecordInterface[30][]; // We start with 30 Rows. + } - public void insertCell(CellValueRecordInterface cell) { - short column = cell.getColumn(); - int row = cell.getRow(); - if (row >= records.length) { - CellValueRecordInterface[][] oldRecords = records; - int newSize = oldRecords.length * 2; - if(newSize= rowCells.length) { - CellValueRecordInterface[] oldRowCells = rowCells; - int newSize = oldRowCells.length * 2; - if(newSize257) newSize=257; // activate? - rowCells = new CellValueRecordInterface[newSize]; - System.arraycopy(oldRowCells, 0, rowCells, 0, oldRowCells.length); - records[row] = rowCells; - } - rowCells[column] = cell; + public void insertCell(CellValueRecordInterface cell) { + short column = cell.getColumn(); + int row = cell.getRow(); + if (row >= records.length) { + CellValueRecordInterface[][] oldRecords = records; + int newSize = oldRecords.length * 2; + if (newSize < row + 1) + newSize = row + 1; + records = new CellValueRecordInterface[newSize][]; + System.arraycopy(oldRecords, 0, records, 0, oldRecords.length); + } + CellValueRecordInterface[] rowCells = records[row]; + if (rowCells == null) { + int newSize = column + 1; + if (newSize < 10) + newSize = 10; + rowCells = new CellValueRecordInterface[newSize]; + records[row] = rowCells; + } + if (column >= rowCells.length) { + CellValueRecordInterface[] oldRowCells = rowCells; + int newSize = oldRowCells.length * 2; + if (newSize < column + 1) + newSize = column + 1; + // if(newSize>257) newSize=257; // activate? + rowCells = new CellValueRecordInterface[newSize]; + System.arraycopy(oldRowCells, 0, rowCells, 0, oldRowCells.length); + records[row] = rowCells; + } + rowCells[column] = cell; - if ((column < firstcell) || (firstcell == -1)) { - firstcell = column; - } - if ((column > lastcell) || (lastcell == -1)) { - lastcell = column; - } - } + if ((column < firstcell) || (firstcell == -1)) { + firstcell = column; + } + if ((column > lastcell) || (lastcell == -1)) { + lastcell = column; + } + } - public void removeCell(CellValueRecordInterface cell) { - if (cell == null) { - throw new IllegalArgumentException("cell must not be null"); - } - int row = cell.getRow(); - if (row >= records.length) { - throw new RuntimeException("cell row is out of range"); - } - CellValueRecordInterface[] rowCells = records[row]; - if (rowCells == null) { - throw new RuntimeException("cell row is already empty"); - } - short column = cell.getColumn(); - if (column >= rowCells.length) { - throw new RuntimeException("cell column is out of range"); - } - rowCells[column] = null; - } + public void removeCell(CellValueRecordInterface cell) { + if (cell == null) { + throw new IllegalArgumentException("cell must not be null"); + } + int row = cell.getRow(); + if (row >= records.length) { + throw new RuntimeException("cell row is out of range"); + } + CellValueRecordInterface[] rowCells = records[row]; + if (rowCells == null) { + throw new RuntimeException("cell row is already empty"); + } + short column = cell.getColumn(); + if (column >= rowCells.length) { + throw new RuntimeException("cell column is out of range"); + } + rowCells[column] = null; + } - public void removeAllCellsValuesForRow(int rowIndex) { - if (rowIndex >= records.length) { - throw new IllegalArgumentException("Specified rowIndex " + rowIndex - + " is outside the allowable range (0.." +records.length + ")"); - } - records[rowIndex] = null; - } + public void removeAllCellsValuesForRow(int rowIndex) { + if (rowIndex < 0 || rowIndex > MAX_ROW_INDEX) { + throw new IllegalArgumentException("Specified rowIndex " + rowIndex + + " 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) + return; + } + + records[rowIndex] = null; + } - public int getPhysicalNumberOfCells() - { - int count=0; - for(int r=0;r endRow) - break; - if ((row >=startRow) && (row <= endRow)) - size += ((RecordBase)cell).getRecordSize(); - } - return size; - } + /** Tallies a count of the size of the cell records + * 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(); + } + } + return size; + } - /** Returns true if the row has cells attached to it */ - public boolean rowHasCells(int row) { - if (row > records.length-1) //previously this said row > records.length which means if - return false; // if records.length == 60 and I pass "60" here I get array out of bounds - CellValueRecordInterface[] rowCells=records[row]; //because a 60 length array has the last index = 59 - if(rowCells==null) return false; - for(int col=0;col= records.length) { + return false; + } + CellValueRecordInterface[] rowCells=records[row]; + if(rowCells==null) return false; + for(int col=0;col temp = new ArrayList(); - for (int i = 0; i < records.length; i++) { - CellValueRecordInterface[] rowCells = records[i]; - if (rowCells == null) { - continue; - } - for (int j = 0; j < rowCells.length; j++) { - CellValueRecordInterface cell = rowCells[j]; - if (cell != null) { - temp.add(cell); - } - } - } + for (int i = 0; i < records.length; i++) { + CellValueRecordInterface[] rowCells = records[i]; + if (rowCells == null) { + continue; + } + for (int j = 0; j < rowCells.length; j++) { + CellValueRecordInterface cell = rowCells[j]; + if (cell != null) { + temp.add(cell); + } + } + } - CellValueRecordInterface[] result = new CellValueRecordInterface[temp.size()]; - temp.toArray(result); - return result; - } - public Iterator getIterator() - { - return new MyIterator(); - } + CellValueRecordInterface[] result = new CellValueRecordInterface[temp.size()]; + temp.toArray(result); + return result; + } + public Iterator getIterator() { + return new MyIterator(records); + } - private final class MyIterator implements Iterator { - short nextColumn=-1; - int nextRow,lastRow; + private static final class MyIterator implements Iterator { + private final CellValueRecordInterface[][] records; + private short nextColumn = -1; + private int nextRow, lastRow; - public MyIterator() - { - this.nextRow=0; - this.lastRow=records.length-1; - findNext(); - } + public MyIterator(CellValueRecordInterface[][] pRecords) { + this(pRecords, 0, pRecords.length - 1); + } - public MyIterator(int firstRow,int lastRow) - { - this.nextRow=firstRow; - this.lastRow=lastRow; - findNext(); - } + 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"); - } + public boolean hasNext() { + return nextRow <= lastRow; + } - 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 records = new ArrayList(); + records.add(new FormulaRecord()); + records.add(new SharedFormulaRecord()); + records.add(new WindowTwoRecord()); - constructValueRecord(records); - Iterator iterator = valueRecord.getIterator(); - RecordBase record = (RecordBase) iterator.next(); - 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() ); + constructValueRecord(records); + Iterator iterator = valueRecord.getIterator(); + RecordBase record = (RecordBase) iterator.next(); + 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) { - RowBlocksReader rbr = new RowBlocksReader(new RecordStream(records, 0)); - SharedValueManager sfrh = rbr.getSharedFormulaManager(); - RecordStream rs = rbr.getPlainRecordStream(); - while(rs.hasNext()) { - Record rec = rs.getNext(); - valueRecord.construct((CellValueRecordInterface)rec, rs, sfrh); - } - } + private void constructValueRecord(List records) { + RowBlocksReader rbr = new RowBlocksReader(new RecordStream(records, 0)); + SharedValueManager sfrh = rbr.getSharedFormulaManager(); + RecordStream rs = rbr.getPlainRecordStream(); + while(rs.hasNext()) { + Record rec = rs.getNext(); + valueRecord.construct((CellValueRecordInterface)rec, rs, sfrh); + } + } - private static List testData() { - List records = new ArrayList(); - FormulaRecord formulaRecord = new FormulaRecord(); - BlankRecord blankRecord = new BlankRecord(); - formulaRecord.setRow( 1 ); - formulaRecord.setColumn( (short) 1 ); - blankRecord.setRow( 2 ); - blankRecord.setColumn( (short) 2 ); - records.add( formulaRecord ); - records.add( blankRecord ); - records.add(new WindowTwoRecord()); - return records; - } + private static List testData() { + List records = new ArrayList(); + FormulaRecord formulaRecord = new FormulaRecord(); + BlankRecord blankRecord = new BlankRecord(); + formulaRecord.setRow(1); + formulaRecord.setColumn((short) 1); + blankRecord.setRow(2); + blankRecord.setColumn((short) 2); + records.add(formulaRecord); + records.add(blankRecord); + records.add(new WindowTwoRecord()); + return records; + } - public void testInsertCell() { - Iterator iterator = valueRecord.getIterator(); - assertFalse( iterator.hasNext() ); + public void testInsertCell() { + Iterator iterator = valueRecord.getIterator(); + assertFalse( iterator.hasNext() ); - BlankRecord blankRecord = newBlankRecord(); - valueRecord.insertCell( blankRecord ); - iterator = valueRecord.getIterator(); - assertTrue( iterator.hasNext() ); - } + BlankRecord blankRecord = newBlankRecord(); + valueRecord.insertCell( blankRecord ); + iterator = valueRecord.getIterator(); + assertTrue( iterator.hasNext() ); + } - public void testRemoveCell() { - BlankRecord blankRecord1 = newBlankRecord(); - valueRecord.insertCell( blankRecord1 ); - BlankRecord blankRecord2 = newBlankRecord(); - valueRecord.removeCell( blankRecord2 ); - Iterator iterator = valueRecord.getIterator(); - assertFalse( iterator.hasNext() ); + public void testRemoveCell() { + BlankRecord blankRecord1 = newBlankRecord(); + valueRecord.insertCell( blankRecord1 ); + BlankRecord blankRecord2 = newBlankRecord(); + valueRecord.removeCell( blankRecord2 ); + Iterator iterator = valueRecord.getIterator(); + assertFalse( iterator.hasNext() ); - // removing an already empty cell just falls through - valueRecord.removeCell( blankRecord2 ); - } + // removing an already empty cell just falls through + valueRecord.removeCell( blankRecord2 ); + } - public void testGetPhysicalNumberOfCells() { - assertEquals(0, valueRecord.getPhysicalNumberOfCells()); - BlankRecord blankRecord1 = newBlankRecord(); - valueRecord.insertCell( blankRecord1 ); - assertEquals(1, valueRecord.getPhysicalNumberOfCells()); - valueRecord.removeCell( blankRecord1 ); - assertEquals(0, valueRecord.getPhysicalNumberOfCells()); - } + public void testGetPhysicalNumberOfCells() { + assertEquals(0, valueRecord.getPhysicalNumberOfCells()); + BlankRecord blankRecord1 = newBlankRecord(); + valueRecord.insertCell( blankRecord1 ); + assertEquals(1, valueRecord.getPhysicalNumberOfCells()); + valueRecord.removeCell( blankRecord1 ); + assertEquals(0, valueRecord.getPhysicalNumberOfCells()); + } - public void testGetFirstCellNum() { - assertEquals( -1, valueRecord.getFirstCellNum() ); - valueRecord.insertCell( newBlankRecord( 2, 2 ) ); - assertEquals( 2, valueRecord.getFirstCellNum() ); - valueRecord.insertCell( newBlankRecord( 3, 3 ) ); - assertEquals( 2, valueRecord.getFirstCellNum() ); + public void testGetFirstCellNum() { + assertEquals( -1, valueRecord.getFirstCellNum() ); + valueRecord.insertCell( newBlankRecord( 2, 2 ) ); + assertEquals( 2, valueRecord.getFirstCellNum() ); + valueRecord.insertCell( newBlankRecord( 3, 3 ) ); + assertEquals( 2, valueRecord.getFirstCellNum() ); - // Note: Removal doesn't currently reset the first column. It probably should but it doesn't. - valueRecord.removeCell( newBlankRecord( 2, 2 ) ); - assertEquals( 2, valueRecord.getFirstCellNum() ); - } + // Note: Removal doesn't currently reset the first column. It probably should but it doesn't. + valueRecord.removeCell( newBlankRecord( 2, 2 ) ); + assertEquals( 2, valueRecord.getFirstCellNum() ); + } - public void testGetLastCellNum() { - assertEquals( -1, valueRecord.getLastCellNum() ); - valueRecord.insertCell( newBlankRecord( 2, 2 ) ); - assertEquals( 2, valueRecord.getLastCellNum() ); - valueRecord.insertCell( newBlankRecord( 3, 3 ) ); - assertEquals( 3, valueRecord.getLastCellNum() ); + public void testGetLastCellNum() { + assertEquals( -1, valueRecord.getLastCellNum() ); + valueRecord.insertCell( newBlankRecord( 2, 2 ) ); + assertEquals( 2, valueRecord.getLastCellNum() ); + valueRecord.insertCell( newBlankRecord( 3, 3 ) ); + assertEquals( 3, valueRecord.getLastCellNum() ); - // Note: Removal doesn't currently reset the last column. It probably should but it doesn't. - valueRecord.removeCell( newBlankRecord( 3, 3 ) ); - assertEquals( 3, valueRecord.getLastCellNum() ); + // Note: Removal doesn't currently reset the last column. It probably should but it doesn't. + valueRecord.removeCell( newBlankRecord( 3, 3 ) ); + assertEquals( 3, valueRecord.getLastCellNum() ); - } + } - 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] ); - } + 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 BlankRecord newBlankRecord() - { - return newBlankRecord( 2, 2 ); - } + private static BlankRecord newBlankRecord() + { + return newBlankRecord( 2, 2 ); + } - private static BlankRecord newBlankRecord( int col, int row) - { - BlankRecord blankRecord = new BlankRecord(); - blankRecord.setRow( row ); - blankRecord.setColumn( (short) col ); - return blankRecord; - } + private static BlankRecord newBlankRecord( int col, int row) + { + BlankRecord blankRecord = new BlankRecord(); + blankRecord.setRow( row ); + blankRecord.setColumn( (short) col ); + return blankRecord; + } - /** - * 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 - * 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) - * 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 - * 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. - *
  2. Sheet3 was deleted.
    3. - *
  3. Sheet2!A1 formula was set to '="second formula"', and fill-dragged through A1:A8.
    4. - *
  4. Sheet1!A1 formula was set to '="first formula"', and also fill-dragged through A1:A8.
    5. - *
  5. Four rows on Sheet1 "5" through "8" were deleted ('delete rows' alt-E D, not 'clear' Del).
    6. - *
  6. The spreadsheet was saved as AbnormalSharedFormulaFlag.xls.
    7. - *
- * 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 - * SharedFormulaRecord from Sheet1 (but not clear the 'shared formula' flags.
- * 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) { - System.err.println("Expected crc " + expectedCRC + " but got " + actualCRC); - 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 - cellFormula = getFormulaFromFirstCell(s, 1); // row "2" - if(cellFormula.equals("\"second formula\"")) { - throw new AssertionFailedError("found bug 44449 (Wrong SharedFormulaRecord was used)."); - } - if(!cellFormula.equals("\"first formula\"")) { - throw new RuntimeException("Something else wrong with this test case"); - } - } - private static String getFormulaFromFirstCell(HSSFSheet s, int rowIx) { - return s.getRow(rowIx).getCell(0).getCellFormula(); - } + /** + * 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 + * 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) + * 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 + * 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. + *
  2. Sheet3 was deleted.
    3. + *
  3. Sheet2!A1 formula was set to '="second formula"', and fill-dragged through A1:A8.
    4. + *
  4. Sheet1!A1 formula was set to '="first formula"', and also fill-dragged through A1:A8.
    5. + *
  5. Four rows on Sheet1 "5" through "8" were deleted ('delete rows' alt-E D, not 'clear' Del).
    6. + *
  6. The spreadsheet was saved as AbnormalSharedFormulaFlag.xls.
    7. + *
+ * 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 + * SharedFormulaRecord from Sheet1 (but not clear the 'shared formula' flags.
+ * 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) { + System.err.println("Expected crc " + expectedCRC + " but got " + actualCRC); + 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 + cellFormula = getFormulaFromFirstCell(s, 1); // row "2" + if(cellFormula.equals("\"second formula\"")) { + throw new AssertionFailedError("found bug 44449 (Wrong SharedFormulaRecord was used)."); + } + if(!cellFormula.equals("\"first formula\"")) { + throw new RuntimeException("Something else wrong with this test case"); + } + } + private static String getFormulaFromFirstCell(HSSFSheet s, int rowIx) { + return s.getRow(rowIx).getCell(0).getCellFormula(); + } - /** - * If someone opened this particular test file in Excel and saved it, the peculiar condition - * which causes the target bug would probably disappear. This test would then just succeed - * regardless of whether the fix was present. So a CRC check is performed to make it less easy - * for that to occur. - */ - private static RuntimeException failUnexpectedTestFileChange() { - String msg = "Test file '" + ABNORMAL_SHARED_FORMULA_FLAG_TEST_FILE + "' has changed. " - + "This junit may not be properly testing for the target bug. " - + "Either revert the test file or ensure that the new version " - + "has the right characteristics to test the target bug."; - // 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). - throw new RuntimeException(msg); - } + /** + * If someone opened this particular test file in Excel and saved it, the peculiar condition + * which causes the target bug would probably disappear. This test would then just succeed + * regardless of whether the fix was present. So a CRC check is performed to make it less easy + * for that to occur. + */ + private static RuntimeException failUnexpectedTestFileChange() { + String msg = "Test file '" + ABNORMAL_SHARED_FORMULA_FLAG_TEST_FILE + "' has changed. " + + "This junit may not be properly testing for the target bug. " + + "Either revert the test file or ensure that the new version " + + "has the right characteristics to test the target bug."; + // 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). + throw new RuntimeException(msg); + } - /** - * gets a CRC checksum for the content of a file - */ - private static long getFileCRC(InputStream is) { - CRC32 crc = new CRC32(); - byte[] buf = new byte[2048]; - try { - while(true) { - int bytesRead = is.read(buf); - if(bytesRead < 1) { - break; - } - crc.update(buf, 0, bytesRead); - } - is.close(); - } catch (IOException e) { - throw new RuntimeException(e); - } - - return crc.getValue(); - } + /** + * gets a CRC checksum for the content of a file + */ + private static long getFileCRC(InputStream is) { + CRC32 crc = new CRC32(); + byte[] buf = new byte[2048]; + try { + while(true) { + int bytesRead = is.read(buf); + if(bytesRead < 1) { + break; + } + crc.update(buf, 0, bytesRead); + } + is.close(); + } 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); + } catch (IllegalArgumentException e) { + if (e.getMessage().equals("Specified rowIndex 30 is outside the allowable range (0..30)")) { + throw new AssertionFailedError("Identified bug 46312"); + } + throw e; + } + + if (false) { // same bug as demonstrated through usermodel API + + HSSFWorkbook wb = new HSSFWorkbook(); + HSSFSheet sheet = wb.createSheet(); + HSSFRow row = sheet.createRow(rowIndex); + if (false) { // must not add any cells to the new row if we want to see the bug + row.createCell(0); // this causes ValueRecordsAggregate.records to auto-extend + } + try { + sheet.createRow(rowIndex); + } catch (IllegalArgumentException e) { + throw new AssertionFailedError("Identified bug 46312"); + } + } + } }