From: Josh Micich Date: Fri, 18 Sep 2009 00:33:18 +0000 (+0000) Subject: Bugzilla 47747 - fixed logic for locating shared formula records X-Git-Tag: REL_3_5-FINAL~2 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=faa8b69a1e0aceae59e1ef9649d468be31d5c6b1;p=poi.git Bugzilla 47747 - fixed logic for locating shared formula records git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@816417 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index c4c9288550..53dbe1e3d6 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -33,8 +33,9 @@ + 47747 - fixed logic for locating shared formula records 47809 - Improved work with user-defined functions - 47581 - fixed XSSFSheet.setColumnWidth to produce XML compatible with Mac Excel 2008 + 47581 - fixed XSSFSheet.setColumnWidth to produce XML compatible with Mac Excel 2008 47734 - removed unnecessary svn:executable flag from files in SVN trunk 47543 - added javadoc how to avoid Excel crash when creating too many HSSFRichTextString cells 47813 - fixed problems with XSSFWorkbook.removeSheetAt when workbook contains chart diff --git a/src/java/org/apache/poi/hssf/model/RowBlocksReader.java b/src/java/org/apache/poi/hssf/model/RowBlocksReader.java index 038304916a..24498267c8 100644 --- a/src/java/org/apache/poi/hssf/model/RowBlocksReader.java +++ b/src/java/org/apache/poi/hssf/model/RowBlocksReader.java @@ -21,17 +21,19 @@ import java.util.ArrayList; import java.util.List; import org.apache.poi.hssf.record.ArrayRecord; +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.SharedFormulaRecord; import org.apache.poi.hssf.record.TableRecord; import org.apache.poi.hssf.record.aggregates.MergedCellsTable; import org.apache.poi.hssf.record.aggregates.SharedValueManager; +import org.apache.poi.ss.util.CellReference; /** - * Segregates the 'Row Blocks' section of a single sheet into plain row/cell records and + * Segregates the 'Row Blocks' section of a single sheet into plain row/cell records and * shared formula records. - * + * * @author Josh Micich */ public final class RowBlocksReader { @@ -47,45 +49,56 @@ public final class RowBlocksReader { public RowBlocksReader(RecordStream rs) { List plainRecords = new ArrayList(); List shFrmRecords = new ArrayList(); + List firstCellRefs = new ArrayList(); List arrayRecords = new ArrayList(); List tableRecords = new ArrayList(); List mergeCellRecords = new ArrayList(); + Record prevRec = null; while(!RecordOrderer.isEndOfRowBlock(rs.peekNextSid())) { // End of row/cell records for the current sheet - // Note - It is important that this code does not inadvertently add any sheet - // records from a subsequent sheet. For example, if SharedFormulaRecords + // Note - It is important that this code does not inadvertently add any sheet + // records from a subsequent sheet. For example, if SharedFormulaRecords // are taken from the wrong sheet, this could cause bug 44449. if (!rs.hasNext()) { throw new RuntimeException("Failed to find end of row/cell records"); - + } Record rec = rs.getNext(); List dest; switch (rec.getSid()) { case MergeCellsRecord.sid: dest = mergeCellRecords; break; - case SharedFormulaRecord.sid: dest = shFrmRecords; break; + case SharedFormulaRecord.sid: dest = shFrmRecords; + if (!(prevRec instanceof FormulaRecord)) { + throw new RuntimeException("Shared formula record should follow a FormulaRecord"); + } + FormulaRecord fr = (FormulaRecord)prevRec; + firstCellRefs.add(new CellReference(fr.getRow(), fr.getColumn())); + break; case ArrayRecord.sid: dest = arrayRecords; break; case TableRecord.sid: dest = tableRecords; break; default: dest = plainRecords; } dest.add(rec); + prevRec = rec; } SharedFormulaRecord[] sharedFormulaRecs = new SharedFormulaRecord[shFrmRecords.size()]; + CellReference[] firstCells = new CellReference[firstCellRefs.size()]; ArrayRecord[] arrayRecs = new ArrayRecord[arrayRecords.size()]; TableRecord[] tableRecs = new TableRecord[tableRecords.size()]; shFrmRecords.toArray(sharedFormulaRecs); + firstCellRefs.toArray(firstCells); arrayRecords.toArray(arrayRecs); tableRecords.toArray(tableRecs); - + _plainRecords = plainRecords; - _sfm = SharedValueManager.create(sharedFormulaRecs, arrayRecs, tableRecs); + _sfm = SharedValueManager.create(sharedFormulaRecs, firstCells, arrayRecs, tableRecs); _mergedCellsRecords = new MergeCellsRecord[mergeCellRecords.size()]; mergeCellRecords.toArray(_mergedCellsRecords); } - + /** - * Some unconventional apps place {@link MergeCellsRecord}s within the row block. They + * Some unconventional apps place {@link MergeCellsRecord}s within the row block. They * actually should be in the {@link MergedCellsTable} which is much later (see bug 45699). * @return any loose MergeCellsRecords found */ @@ -97,7 +110,7 @@ public final class RowBlocksReader { return _sfm; } /** - * @return a {@link RecordStream} containing all the non-{@link SharedFormulaRecord} + * @return a {@link RecordStream} containing all the non-{@link SharedFormulaRecord} * non-{@link ArrayRecord} and non-{@link TableRecord} Records. */ public RecordStream getPlainRecordStream() { diff --git a/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java b/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java index 73869e193b..0f4e976d9a 100644 --- a/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java +++ b/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java @@ -58,10 +58,12 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel } else { // Usually stringRec is null here (in agreement with what the formula rec says). // In the case where an extra StringRecord is erroneously present, Excel (2007) - // ignores it (see bug 46213). + // ignores it (see bug 46213). _stringRecord = null; } + _formulaRecord = formulaRec; + _sharedValueManager = svm; if (formulaRec.isSharedFormula()) { CellReference firstCell = formulaRec.getFormula().getExpReference(); if (firstCell == null) { @@ -70,24 +72,22 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel _sharedFormulaRecord = svm.linkSharedFormulaRecord(firstCell, this); } } - _formulaRecord = formulaRec; - _sharedValueManager = svm; } /** * Sometimes the shared formula flag "seems" to be erroneously set (because the corresponding * {@link SharedFormulaRecord} does not exist). Normally this would leave no way of determining - * the {@link Ptg} tokens for the formula. However as it turns out in these + * the {@link Ptg} tokens for the formula. However as it turns out in these * cases, Excel encodes the unshared {@link Ptg} tokens in the right place (inside the {@link * FormulaRecord}). So the the only thing that needs to be done is to ignore the erroneous * shared formula flag.
- * + * * This method may also be used for setting breakpoints to help diagnose issues regarding the - * abnormally-set 'shared formula' flags. + * abnormally-set 'shared formula' flags. * (see TestValueRecordsAggregate.testSpuriousSharedFormulaFlag()).

*/ private static void handleMissingSharedFormulaRecord(FormulaRecord formula) { // make sure 'unshared' formula is actually available - Ptg firstToken = formula.getParsedExpression()[0]; + Ptg firstToken = formula.getParsedExpression()[0]; if (firstToken instanceof ExpPtg) { throw new RecordFormatException( "SharedFormulaRecord not found for FormulaRecord with (isSharedFormula=true)"); @@ -138,14 +138,9 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel public void visitContainedRecords(RecordVisitor rv) { rv.visitRecord(_formulaRecord); - CellReference sharedFirstCell = _formulaRecord.getFormula().getExpReference(); - // perhaps this could be optimised by consulting the (somewhat unreliable) isShared flag - // and/or distinguishing between tExp and tTbl. - if (sharedFirstCell != null) { - Record sharedFormulaRecord = _sharedValueManager.getRecordForFirstCell(sharedFirstCell, this); - if (sharedFormulaRecord != null) { - rv.visitRecord(sharedFormulaRecord); - } + Record sharedFormulaRecord = _sharedValueManager.getRecordForFirstCell(this); + if (sharedFormulaRecord != null) { + rv.visitRecord(sharedFormulaRecord); } if (_formulaRecord.hasCachedResultString() && _stringRecord != null) { rv.visitRecord(_stringRecord); diff --git a/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java b/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java index f007ebe6e9..8c4d19d2b8 100644 --- a/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java +++ b/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java @@ -28,9 +28,8 @@ 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.formula.ExpPtg; -import org.apache.poi.hssf.record.formula.TblPtg; import org.apache.poi.hssf.util.CellRangeAddress8Bit; -import org.apache.poi.hssf.util.CellReference; +import org.apache.poi.ss.util.CellReference; /** * Manages various auxiliary records while constructing a @@ -45,26 +44,38 @@ import org.apache.poi.hssf.util.CellReference; */ public final class SharedValueManager { - // This class should probably be generalised to handle array and table groups too - private static final class SharedValueGroup { - private final SharedValueRecordBase _svr; - private FormulaRecordAggregate[] _frAggs; + private static final class SharedFormulaGroup { + private final SharedFormulaRecord _sfr; + private final FormulaRecordAggregate[] _frAggs; private int _numberOfFormulas; + /** + * Coordinates of the first cell having a formula that uses this shared formula. + * This is often but not always the top left cell in the range covered by + * {@link #_sfr} + */ + private final CellReference _firstCell; - public SharedValueGroup(SharedValueRecordBase svr) { - _svr = svr; - int width = svr.getLastColumn() - svr.getFirstColumn() + 1; - int height = svr.getLastRow() - svr.getFirstRow() + 1; + public SharedFormulaGroup(SharedFormulaRecord sfr, CellReference firstCell) { + if (!sfr.isInRange(firstCell.getRow(), firstCell.getCol())) { + throw new IllegalArgumentException("First formula cell " + firstCell.formatAsString() + + " is not shared formula range " + sfr.getRange().toString() + "."); + } + _sfr = sfr; + _firstCell = firstCell; + int width = sfr.getLastColumn() - sfr.getFirstColumn() + 1; + int height = sfr.getLastRow() - sfr.getFirstRow() + 1; _frAggs = new FormulaRecordAggregate[width * height]; _numberOfFormulas = 0; } public void add(FormulaRecordAggregate agg) { + if (_numberOfFormulas == 0) { + if (_firstCell.getRow() != agg.getRow() || _firstCell.getCol() != agg.getColumn()) { + throw new IllegalStateException("shared formula coding error"); + } + } if (_numberOfFormulas >= _frAggs.length) { - // this probably shouldn't occur - problems with sample file "15228.xls" - FormulaRecordAggregate[] temp = new FormulaRecordAggregate[_numberOfFormulas*2]; - System.arraycopy(_frAggs, 0, temp, 0, _frAggs.length); - _frAggs = temp; + throw new RuntimeException("Too many formula records for shared formula group"); } _frAggs[_numberOfFormulas++] = agg; } @@ -75,49 +86,55 @@ public final class SharedValueManager { } } - public SharedValueRecordBase getSVR() { - return _svr; + public SharedFormulaRecord getSFR() { + return _sfr; } - /** - * Note - Sometimes the first formula in a group is not present (because the range - * is sparsely populated), so this method can return true for a cell - * that is not the top-left corner of the range. - * @return true if this is the first formula cell in the group - */ - public boolean isFirstMember(FormulaRecordAggregate agg) { - return agg == _frAggs[0]; - } public final String toString() { StringBuffer sb = new StringBuffer(64); sb.append(getClass().getName()).append(" ["); - sb.append(_svr.getRange().toString()); + sb.append(_sfr.getRange().toString()); sb.append("]"); return sb.toString(); } + + /** + * Note - the 'first cell' of a shared formula group is not always the top-left cell + * of the enclosing range. + * @return true if the specified coordinates correspond to the 'first cell' + * of this shared formula group. + */ + public boolean isFirstCell(int row, int column) { + return _firstCell.getRow() == row && _firstCell.getCol() == column; + } } public static final SharedValueManager EMPTY = new SharedValueManager( - new SharedFormulaRecord[0], new ArrayRecord[0], new TableRecord[0]); + new SharedFormulaRecord[0], new CellReference[0], new ArrayRecord[0], new TableRecord[0]); private final ArrayRecord[] _arrayRecords; private final TableRecord[] _tableRecords; - private final Map _groupsBySharedFormulaRecord; + private final Map _groupsBySharedFormulaRecord; /** cached for optimization purposes */ - private SharedValueGroup[] _groups; + private SharedFormulaGroup[] _groups; private SharedValueManager(SharedFormulaRecord[] sharedFormulaRecords, - ArrayRecord[] arrayRecords, TableRecord[] tableRecords) { + CellReference[] firstCells, ArrayRecord[] arrayRecords, TableRecord[] tableRecords) { + int nShF = sharedFormulaRecords.length; + if (nShF != firstCells.length) { + throw new IllegalArgumentException("array sizes don't match: " + nShF + "!=" + firstCells.length + "."); + } _arrayRecords = arrayRecords; _tableRecords = tableRecords; - Map m = new HashMap(sharedFormulaRecords.length * 3 / 2); - for (int i = 0; i < sharedFormulaRecords.length; i++) { + Map m = new HashMap(nShF * 3 / 2); + for (int i = 0; i < nShF; i++) { SharedFormulaRecord sfr = sharedFormulaRecords[i]; - m.put(sfr, new SharedValueGroup(sfr)); + m.put(sfr, new SharedFormulaGroup(sfr, firstCells[i])); } _groupsBySharedFormulaRecord = m; } /** + * @param firstCells * @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 @@ -125,11 +142,11 @@ public final class SharedValueManager { * sheet (which could happen if endIx is chosen poorly). (see bug 44449) */ public static SharedValueManager create(SharedFormulaRecord[] sharedFormulaRecords, - ArrayRecord[] arrayRecords, TableRecord[] tableRecords) { - if (sharedFormulaRecords.length + arrayRecords.length + tableRecords.length < 1) { + CellReference[] firstCells, ArrayRecord[] arrayRecords, TableRecord[] tableRecords) { + if (sharedFormulaRecords.length + firstCells.length + arrayRecords.length + tableRecords.length < 1) { return EMPTY; } - return new SharedValueManager(sharedFormulaRecords, arrayRecords, tableRecords); + return new SharedValueManager(sharedFormulaRecords, firstCells, arrayRecords, tableRecords); } @@ -139,68 +156,30 @@ public final class SharedValueManager { */ public SharedFormulaRecord linkSharedFormulaRecord(CellReference firstCell, FormulaRecordAggregate agg) { - SharedValueGroup result = findGroup(getGroups(), firstCell); + SharedFormulaGroup result = findFormulaGroup(getGroups(), firstCell); result.add(agg); - return (SharedFormulaRecord) result.getSVR(); + return result.getSFR(); } - private static SharedValueGroup findGroup(SharedValueGroup[] groups, CellReference firstCell) { + private static SharedFormulaGroup findFormulaGroup(SharedFormulaGroup[] groups, CellReference firstCell) { int row = firstCell.getRow(); int column = firstCell.getCol(); // Traverse the list of shared formulas and try to find the correct one for us // perhaps this could be optimised to some kind of binary search for (int i = 0; i < groups.length; i++) { - SharedValueGroup svg = groups[i]; - if (svg.getSVR().isFirstCell(row, column)) { + SharedFormulaGroup svg = groups[i]; + if (svg.isFirstCell(row, column)) { return svg; } } - // else - no SharedFormulaRecord was found with the specified firstCell. - // This is unusual, but one sample file exhibits the anomaly: "ex45046-21984.xls" - // Excel seems to handle the problem OK, and doesn't even correct it. Perhaps POI should. - - // search for shared formula by range - SharedValueGroup result = null; - for (int i = 0; i < groups.length; i++) { - SharedValueGroup svg = groups[i]; - if (svg.getSVR().isInRange(row, column)) { - if (result != null) { - // This happens in sample file "15228.xls" - if (sharedFormulasAreSame(result, svg)) { - // hopefully this is OK - just use the first one since they are the same - // not quite - // TODO - fix file "15228.xls" so it opens in Excel after rewriting with POI - } else { - throw new RuntimeException("This cell is in the range of more than one distinct shared formula"); - } - } else { - result = svg; - } - } - } - if (result == null) { - throw new RuntimeException("Failed to find a matching shared formula record"); - } - return result; - } - - /** - * Handles the ugly situation (seen in example "15228.xls") where a shared formula cell is - * covered by more than one shared formula range, but the formula cell's {@link ExpPtg} - * doesn't identify any of them. - * @return true if the underlying shared formulas are the same - */ - private static boolean sharedFormulasAreSame(SharedValueGroup grpA, SharedValueGroup grpB) { - // safe to cast here because this findGroup() is never called for ARRAY or TABLE formulas - SharedFormulaRecord sfrA = (SharedFormulaRecord) grpA.getSVR(); - SharedFormulaRecord sfrB = (SharedFormulaRecord) grpB.getSVR(); - return sfrA.isFormulaSame(sfrB); + // TODO - fix file "15228.xls" so it opens in Excel after rewriting with POI + throw new RuntimeException("Failed to find a matching shared formula record"); } - private SharedValueGroup[] getGroups() { + private SharedFormulaGroup[] getGroups() { if (_groups == null) { - SharedValueGroup[] groups = new SharedValueGroup[_groupsBySharedFormulaRecord.size()]; + SharedFormulaGroup[] groups = new SharedFormulaGroup[_groupsBySharedFormulaRecord.size()]; _groupsBySharedFormulaRecord.values().toArray(groups); Arrays.sort(groups, SVGComparator); // make search behaviour more deterministic _groups = groups; @@ -208,11 +187,11 @@ public final class SharedValueManager { return _groups; } - private static final Comparator SVGComparator = new Comparator() { + private static final Comparator SVGComparator = new Comparator() { - public int compare(Object a, Object b) { - CellRangeAddress8Bit rangeA = ((SharedValueGroup)a).getSVR().getRange(); - CellRangeAddress8Bit rangeB = ((SharedValueGroup)b).getSVR().getRange(); + public int compare(SharedFormulaGroup a, SharedFormulaGroup b) { + CellRangeAddress8Bit rangeA = a.getSFR().getRange(); + CellRangeAddress8Bit rangeB = b.getSFR().getRange(); int cmp; cmp = rangeA.getFirstRow() - rangeB.getFirstRow(); @@ -228,44 +207,57 @@ public final class SharedValueManager { }; /** - * The {@link SharedValueRecordBase} record returned by this method - * @param firstCell the cell coordinates as read from the {@link ExpPtg} or {@link TblPtg} - * of the current formula. Note - this is usually not the same as the cell coordinates - * of the formula's cell. + * Gets the {@link SharedValueRecordBase} record if it should be encoded immediately after the + * formula record contained in the specified {@link FormulaRecordAggregate} agg. Note - the + * shared value record always appears after the first formula record in the group. For arrays + * and tables the first formula is always the in the top left cell. However, since shared + * formula groups can be sparse and/or overlap, the first formula may not actually be in the + * top left cell. * - * @return the SHRFMLA, TABLE or ARRAY record for this formula cell, if it is the first cell of a - * table or array region. null if + * @return the SHRFMLA, TABLE or ARRAY record for the formula cell, if it is the first cell of + * a table or array region. null if the formula cell is not shared/array/table, + * or if the specified formula is not the the first in the group. */ - public SharedValueRecordBase getRecordForFirstCell(CellReference firstCell, FormulaRecordAggregate agg) { + public SharedValueRecordBase getRecordForFirstCell(FormulaRecordAggregate agg) { + CellReference firstCell = agg.getFormulaRecord().getFormula().getExpReference(); + // perhaps this could be optimised by consulting the (somewhat unreliable) isShared flag + // and/or distinguishing between tExp and tTbl. + if (firstCell == null) { + // not a shared/array/table formula + return null; + } + + int row = firstCell.getRow(); int column = firstCell.getCol(); - boolean isTopLeft = agg.getRow() == row && agg.getColumn() == column; - if (isTopLeft) { - for (int i = 0; i < _tableRecords.length; i++) { - TableRecord tr = _tableRecords[i]; - if (tr.isFirstCell(row, column)) { - return tr; - } + if (agg.getRow() != row || agg.getColumn() != column) { + // not the first formula cell in the group + return null; + } + SharedFormulaGroup[] groups = getGroups(); + for (int i = 0; i < groups.length; i++) { + // note - logic for finding correct shared formula group is slightly + // more complicated since the first cell + SharedFormulaGroup sfg = groups[i]; + if (sfg.isFirstCell(row, column)) { + return sfg.getSFR(); } - for (int i = 0; i < _arrayRecords.length; i++) { - ArrayRecord ar = _arrayRecords[i]; - if (ar.isFirstCell(row, column)) { - return ar; - } + } + + // Since arrays and tables cannot be sparse (all cells in range participate) + // The first cell will be the top left in the range. So we can match the + // ARRAY/TABLE record directly. + + for (int i = 0; i < _tableRecords.length; i++) { + TableRecord tr = _tableRecords[i]; + if (tr.isFirstCell(row, column)) { + return tr; } - } else { - // Since arrays and tables cannot be sparse (all cells in range participate) - // no need to search arrays and tables if agg is not the top left cell } - SharedValueGroup[] groups = getGroups(); - for (int i = 0; i < groups.length; i++) { - SharedValueGroup svg = groups[i]; - SharedValueRecordBase svr = svg.getSVR(); - if (svr.isFirstCell(row, column)) { - if (svg.isFirstMember(agg)) { - return svr; - } - return null; + for (int i = 0; i < _arrayRecords.length; i++) { + ArrayRecord ar = _arrayRecords[i]; + if (ar.isFirstCell(row, column)) { + return ar; } } return null; @@ -276,7 +268,7 @@ public final class SharedValueManager { * to plain unshared formulas */ public void unlink(SharedFormulaRecord sharedFormulaRecord) { - SharedValueGroup svg = (SharedValueGroup) _groupsBySharedFormulaRecord.remove(sharedFormulaRecord); + SharedFormulaGroup svg = _groupsBySharedFormulaRecord.remove(sharedFormulaRecord); _groups = null; // be sure to reset cached value if (svg == null) { throw new IllegalStateException("Failed to find formulas for shared formula"); diff --git a/src/testcases/org/apache/poi/hssf/record/aggregates/TestSharedValueManager.java b/src/testcases/org/apache/poi/hssf/record/aggregates/TestSharedValueManager.java index 9f169cada8..66e1c84f03 100644 --- a/src/testcases/org/apache/poi/hssf/record/aggregates/TestSharedValueManager.java +++ b/src/testcases/org/apache/poi/hssf/record/aggregates/TestSharedValueManager.java @@ -26,6 +26,7 @@ import junit.framework.TestCase; import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.hssf.record.Record; import org.apache.poi.hssf.record.SharedFormulaRecord; +import org.apache.poi.hssf.usermodel.HSSFCell; import org.apache.poi.hssf.usermodel.HSSFSheet; import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.hssf.usermodel.RecordInspector; @@ -120,4 +121,52 @@ public final class TestSharedValueManager extends TestCase { } assertEquals(2, count); } + + /** + * Tests fix for a bug in the way shared formula cells are associated with shared formula + * records. Prior to this fix, POI would attempt to use the upper left corner of the + * shared formula range as the locator cell. The correct cell to use is the 'first cell' + * in the shared formula group which is not always the top left cell. This is possible + * because shared formula groups may be sparse and may overlap.
+ * + * Two existing sample files (15228.xls and ex45046-21984.xls) had similar issues. + * These were not explored fully, but seem to be fixed now. + */ + public void testRecalculateFormulas47747() { + + /* + * ex47747-sharedFormula.xls is a heavily cut-down version of the spreadsheet from + * the attachment (id=24176) in Bugzilla 47747. This was done to make the sample + * file smaller, which hopefully allows the special data encoding condition to be + * seen more easily. Care must be taken when modifying this file since the + * special conditions are easily destroyed (which would make this test useless). + * It seems that removing the worksheet protection has made this more so - if the + * current file is re-saved in Excel(2007) the bug condition disappears. + * + * + * Using BiffViewer, one can see that there are two shared formula groups representing + * the essentially same formula over ~20 cells. The shared group ranges overlap and + * are A12:Q20 and A20:Q27. The locator cell ('first cell') for the second group is + * Q20 which is not the top left cell of the enclosing range. It is this specific + * condition which caused the bug to occur + */ + HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex47747-sharedFormula.xls"); + + // pick out a cell from within the second shared formula group + HSSFCell cell = wb.getSheetAt(0).getRow(23).getCell(0); + String formulaText; + try { + formulaText = cell.getCellFormula(); + // succeeds if the formula record has been associated + // with the second shared formula group + } catch (RuntimeException e) { + // bug occurs if the formula record has been associated + // with the first shared formula group + if ("Shared Formula Conversion: Coding Error".equals(e.getMessage())) { + throw new AssertionFailedError("Identified bug 47747"); + } + throw e; + } + assertEquals("$AF24*A$7", formulaText); + } } diff --git a/test-data/spreadsheet/ex47747-sharedFormula.xls b/test-data/spreadsheet/ex47747-sharedFormula.xls new file mode 100755 index 0000000000..716ffa10c3 Binary files /dev/null and b/test-data/spreadsheet/ex47747-sharedFormula.xls differ