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
This commit is contained in:
Josh Micich 2009-09-18 00:33:18 +00:00
parent cae73922a6
commit c098ef803e
6 changed files with 197 additions and 147 deletions

View File

@ -33,8 +33,9 @@
<changes> <changes>
<release version="3.5-beta7" date="2009-??-??"> <release version="3.5-beta7" date="2009-??-??">
<action dev="POI-DEVELOPERS" type="fix">47747 - fixed logic for locating shared formula records</action>
<action dev="POI-DEVELOPERS" type="add">47809 - Improved work with user-defined functions</action> <action dev="POI-DEVELOPERS" type="add">47809 - Improved work with user-defined functions</action>
<action dev="POI-DEVELOPERS" type="fix">47581 - fixed XSSFSheet.setColumnWidth to produce XML compatible with Mac Excel 2008</action> <action dev="POI-DEVELOPERS" type="fix">47581 - fixed XSSFSheet.setColumnWidth to produce XML compatible with Mac Excel 2008</action>
<action dev="POI-DEVELOPERS" type="fix">47734 - removed unnecessary svn:executable flag from files in SVN trunk</action> <action dev="POI-DEVELOPERS" type="fix">47734 - removed unnecessary svn:executable flag from files in SVN trunk</action>
<action dev="POI-DEVELOPERS" type="fix">47543 - added javadoc how to avoid Excel crash when creating too many HSSFRichTextString cells</action> <action dev="POI-DEVELOPERS" type="fix">47543 - added javadoc how to avoid Excel crash when creating too many HSSFRichTextString cells</action>
<action dev="POI-DEVELOPERS" type="fix">47813 - fixed problems with XSSFWorkbook.removeSheetAt when workbook contains chart</action> <action dev="POI-DEVELOPERS" type="fix">47813 - fixed problems with XSSFWorkbook.removeSheetAt when workbook contains chart</action>

View File

@ -21,12 +21,14 @@ import java.util.ArrayList;
import java.util.List; import java.util.List;
import org.apache.poi.hssf.record.ArrayRecord; 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.MergeCellsRecord;
import org.apache.poi.hssf.record.Record; import org.apache.poi.hssf.record.Record;
import org.apache.poi.hssf.record.SharedFormulaRecord; import org.apache.poi.hssf.record.SharedFormulaRecord;
import org.apache.poi.hssf.record.TableRecord; import org.apache.poi.hssf.record.TableRecord;
import org.apache.poi.hssf.record.aggregates.MergedCellsTable; import org.apache.poi.hssf.record.aggregates.MergedCellsTable;
import org.apache.poi.hssf.record.aggregates.SharedValueManager; 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
@ -47,10 +49,12 @@ public final class RowBlocksReader {
public RowBlocksReader(RecordStream rs) { public RowBlocksReader(RecordStream rs) {
List<Record> plainRecords = new ArrayList<Record>(); List<Record> plainRecords = new ArrayList<Record>();
List<Record> shFrmRecords = new ArrayList<Record>(); List<Record> shFrmRecords = new ArrayList<Record>();
List<CellReference> firstCellRefs = new ArrayList<CellReference>();
List<Record> arrayRecords = new ArrayList<Record>(); List<Record> arrayRecords = new ArrayList<Record>();
List<Record> tableRecords = new ArrayList<Record>(); List<Record> tableRecords = new ArrayList<Record>();
List<Record> mergeCellRecords = new ArrayList<Record>(); List<Record> mergeCellRecords = new ArrayList<Record>();
Record prevRec = null;
while(!RecordOrderer.isEndOfRowBlock(rs.peekNextSid())) { while(!RecordOrderer.isEndOfRowBlock(rs.peekNextSid())) {
// End of row/cell records for the current sheet // End of row/cell records for the current sheet
// Note - It is important that this code does not inadvertently add any sheet // Note - It is important that this code does not inadvertently add any sheet
@ -64,22 +68,31 @@ public final class RowBlocksReader {
List<Record> dest; List<Record> dest;
switch (rec.getSid()) { switch (rec.getSid()) {
case MergeCellsRecord.sid: dest = mergeCellRecords; break; 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 ArrayRecord.sid: dest = arrayRecords; break;
case TableRecord.sid: dest = tableRecords; break; case TableRecord.sid: dest = tableRecords; break;
default: dest = plainRecords; default: dest = plainRecords;
} }
dest.add(rec); dest.add(rec);
prevRec = rec;
} }
SharedFormulaRecord[] sharedFormulaRecs = new SharedFormulaRecord[shFrmRecords.size()]; SharedFormulaRecord[] sharedFormulaRecs = new SharedFormulaRecord[shFrmRecords.size()];
CellReference[] firstCells = new CellReference[firstCellRefs.size()];
ArrayRecord[] arrayRecs = new ArrayRecord[arrayRecords.size()]; ArrayRecord[] arrayRecs = new ArrayRecord[arrayRecords.size()];
TableRecord[] tableRecs = new TableRecord[tableRecords.size()]; TableRecord[] tableRecs = new TableRecord[tableRecords.size()];
shFrmRecords.toArray(sharedFormulaRecs); shFrmRecords.toArray(sharedFormulaRecs);
firstCellRefs.toArray(firstCells);
arrayRecords.toArray(arrayRecs); arrayRecords.toArray(arrayRecs);
tableRecords.toArray(tableRecs); tableRecords.toArray(tableRecs);
_plainRecords = plainRecords; _plainRecords = plainRecords;
_sfm = SharedValueManager.create(sharedFormulaRecs, arrayRecs, tableRecs); _sfm = SharedValueManager.create(sharedFormulaRecs, firstCells, arrayRecs, tableRecs);
_mergedCellsRecords = new MergeCellsRecord[mergeCellRecords.size()]; _mergedCellsRecords = new MergeCellsRecord[mergeCellRecords.size()];
mergeCellRecords.toArray(_mergedCellsRecords); mergeCellRecords.toArray(_mergedCellsRecords);
} }

View File

@ -62,6 +62,8 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel
_stringRecord = null; _stringRecord = null;
} }
_formulaRecord = formulaRec;
_sharedValueManager = svm;
if (formulaRec.isSharedFormula()) { if (formulaRec.isSharedFormula()) {
CellReference firstCell = formulaRec.getFormula().getExpReference(); CellReference firstCell = formulaRec.getFormula().getExpReference();
if (firstCell == null) { if (firstCell == null) {
@ -70,8 +72,6 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel
_sharedFormulaRecord = svm.linkSharedFormulaRecord(firstCell, this); _sharedFormulaRecord = svm.linkSharedFormulaRecord(firstCell, this);
} }
} }
_formulaRecord = formulaRec;
_sharedValueManager = svm;
} }
/** /**
* Sometimes the shared formula flag "seems" to be erroneously set (because the corresponding * Sometimes the shared formula flag "seems" to be erroneously set (because the corresponding
@ -138,14 +138,9 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel
public void visitContainedRecords(RecordVisitor rv) { public void visitContainedRecords(RecordVisitor rv) {
rv.visitRecord(_formulaRecord); rv.visitRecord(_formulaRecord);
CellReference sharedFirstCell = _formulaRecord.getFormula().getExpReference(); Record sharedFormulaRecord = _sharedValueManager.getRecordForFirstCell(this);
// perhaps this could be optimised by consulting the (somewhat unreliable) isShared flag if (sharedFormulaRecord != null) {
// and/or distinguishing between tExp and tTbl. rv.visitRecord(sharedFormulaRecord);
if (sharedFirstCell != null) {
Record sharedFormulaRecord = _sharedValueManager.getRecordForFirstCell(sharedFirstCell, this);
if (sharedFormulaRecord != null) {
rv.visitRecord(sharedFormulaRecord);
}
} }
if (_formulaRecord.hasCachedResultString() && _stringRecord != null) { if (_formulaRecord.hasCachedResultString() && _stringRecord != null) {
rv.visitRecord(_stringRecord); rv.visitRecord(_stringRecord);

View File

@ -28,9 +28,8 @@ import org.apache.poi.hssf.record.SharedFormulaRecord;
import org.apache.poi.hssf.record.SharedValueRecordBase; import org.apache.poi.hssf.record.SharedValueRecordBase;
import org.apache.poi.hssf.record.TableRecord; import org.apache.poi.hssf.record.TableRecord;
import org.apache.poi.hssf.record.formula.ExpPtg; 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.CellRangeAddress8Bit;
import org.apache.poi.hssf.util.CellReference; import org.apache.poi.ss.util.CellReference;
/** /**
* Manages various auxiliary records while constructing a * Manages various auxiliary records while constructing a
@ -45,26 +44,38 @@ import org.apache.poi.hssf.util.CellReference;
*/ */
public final class SharedValueManager { public final class SharedValueManager {
// This class should probably be generalised to handle array and table groups too private static final class SharedFormulaGroup {
private static final class SharedValueGroup { private final SharedFormulaRecord _sfr;
private final SharedValueRecordBase _svr; private final FormulaRecordAggregate[] _frAggs;
private FormulaRecordAggregate[] _frAggs;
private int _numberOfFormulas; private int _numberOfFormulas;
/**
* Coordinates of the first cell having a formula that uses this shared formula.
* This is often <i>but not always</i> the top left cell in the range covered by
* {@link #_sfr}
*/
private final CellReference _firstCell;
public SharedValueGroup(SharedValueRecordBase svr) { public SharedFormulaGroup(SharedFormulaRecord sfr, CellReference firstCell) {
_svr = svr; if (!sfr.isInRange(firstCell.getRow(), firstCell.getCol())) {
int width = svr.getLastColumn() - svr.getFirstColumn() + 1; throw new IllegalArgumentException("First formula cell " + firstCell.formatAsString()
int height = svr.getLastRow() - svr.getFirstRow() + 1; + " 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]; _frAggs = new FormulaRecordAggregate[width * height];
_numberOfFormulas = 0; _numberOfFormulas = 0;
} }
public void add(FormulaRecordAggregate agg) { 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) { if (_numberOfFormulas >= _frAggs.length) {
// this probably shouldn't occur - problems with sample file "15228.xls" throw new RuntimeException("Too many formula records for shared formula group");
FormulaRecordAggregate[] temp = new FormulaRecordAggregate[_numberOfFormulas*2];
System.arraycopy(_frAggs, 0, temp, 0, _frAggs.length);
_frAggs = temp;
} }
_frAggs[_numberOfFormulas++] = agg; _frAggs[_numberOfFormulas++] = agg;
} }
@ -75,49 +86,55 @@ public final class SharedValueManager {
} }
} }
public SharedValueRecordBase getSVR() { public SharedFormulaRecord getSFR() {
return _svr; return _sfr;
} }
/**
* Note - Sometimes the first formula in a group is not present (because the range
* is sparsely populated), so this method can return <code>true</code> for a cell
* that is not the top-left corner of the range.
* @return <code>true</code> if this is the first formula cell in the group
*/
public boolean isFirstMember(FormulaRecordAggregate agg) {
return agg == _frAggs[0];
}
public final String toString() { public final String toString() {
StringBuffer sb = new StringBuffer(64); StringBuffer sb = new StringBuffer(64);
sb.append(getClass().getName()).append(" ["); sb.append(getClass().getName()).append(" [");
sb.append(_svr.getRange().toString()); sb.append(_sfr.getRange().toString());
sb.append("]"); sb.append("]");
return sb.toString(); return sb.toString();
} }
/**
* Note - the 'first cell' of a shared formula group is not always the top-left cell
* of the enclosing range.
* @return <code>true</code> 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( 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 ArrayRecord[] _arrayRecords;
private final TableRecord[] _tableRecords; private final TableRecord[] _tableRecords;
private final Map _groupsBySharedFormulaRecord; private final Map<SharedFormulaRecord, SharedFormulaGroup> _groupsBySharedFormulaRecord;
/** cached for optimization purposes */ /** cached for optimization purposes */
private SharedValueGroup[] _groups; private SharedFormulaGroup[] _groups;
private SharedValueManager(SharedFormulaRecord[] sharedFormulaRecords, 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; _arrayRecords = arrayRecords;
_tableRecords = tableRecords; _tableRecords = tableRecords;
Map m = new HashMap(sharedFormulaRecords.length * 3 / 2); Map<SharedFormulaRecord, SharedFormulaGroup> m = new HashMap<SharedFormulaRecord, SharedFormulaGroup>(nShF * 3 / 2);
for (int i = 0; i < sharedFormulaRecords.length; i++) { for (int i = 0; i < nShF; i++) {
SharedFormulaRecord sfr = sharedFormulaRecords[i]; SharedFormulaRecord sfr = sharedFormulaRecords[i];
m.put(sfr, new SharedValueGroup(sfr)); m.put(sfr, new SharedFormulaGroup(sfr, firstCells[i]));
} }
_groupsBySharedFormulaRecord = m; _groupsBySharedFormulaRecord = m;
} }
/** /**
* @param firstCells
* @param recs list of sheet records (possibly contains records for other parts of the Excel file) * @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 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 * @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) * sheet (which could happen if endIx is chosen poorly). (see bug 44449)
*/ */
public static SharedValueManager create(SharedFormulaRecord[] sharedFormulaRecords, public static SharedValueManager create(SharedFormulaRecord[] sharedFormulaRecords,
ArrayRecord[] arrayRecords, TableRecord[] tableRecords) { CellReference[] firstCells, ArrayRecord[] arrayRecords, TableRecord[] tableRecords) {
if (sharedFormulaRecords.length + arrayRecords.length + tableRecords.length < 1) { if (sharedFormulaRecords.length + firstCells.length + arrayRecords.length + tableRecords.length < 1) {
return EMPTY; 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) { public SharedFormulaRecord linkSharedFormulaRecord(CellReference firstCell, FormulaRecordAggregate agg) {
SharedValueGroup result = findGroup(getGroups(), firstCell); SharedFormulaGroup result = findFormulaGroup(getGroups(), firstCell);
result.add(agg); 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 row = firstCell.getRow();
int column = firstCell.getCol(); int column = firstCell.getCol();
// Traverse the list of shared formulas and try to find the correct one for us // 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 // perhaps this could be optimised to some kind of binary search
for (int i = 0; i < groups.length; i++) { for (int i = 0; i < groups.length; i++) {
SharedValueGroup svg = groups[i]; SharedFormulaGroup svg = groups[i];
if (svg.getSVR().isFirstCell(row, column)) { if (svg.isFirstCell(row, column)) {
return svg; return svg;
} }
} }
// else - no SharedFormulaRecord was found with the specified firstCell. // TODO - fix file "15228.xls" so it opens in Excel after rewriting with POI
// This is unusual, but one sample file exhibits the anomaly: "ex45046-21984.xls" throw new RuntimeException("Failed to find a matching shared formula record");
// 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;
} }
/** private SharedFormulaGroup[] getGroups() {
* 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 <code>true</code> 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);
}
private SharedValueGroup[] getGroups() {
if (_groups == null) { if (_groups == null) {
SharedValueGroup[] groups = new SharedValueGroup[_groupsBySharedFormulaRecord.size()]; SharedFormulaGroup[] groups = new SharedFormulaGroup[_groupsBySharedFormulaRecord.size()];
_groupsBySharedFormulaRecord.values().toArray(groups); _groupsBySharedFormulaRecord.values().toArray(groups);
Arrays.sort(groups, SVGComparator); // make search behaviour more deterministic Arrays.sort(groups, SVGComparator); // make search behaviour more deterministic
_groups = groups; _groups = groups;
@ -208,11 +187,11 @@ public final class SharedValueManager {
return _groups; return _groups;
} }
private static final Comparator SVGComparator = new Comparator() { private static final Comparator<SharedFormulaGroup> SVGComparator = new Comparator<SharedFormulaGroup>() {
public int compare(Object a, Object b) { public int compare(SharedFormulaGroup a, SharedFormulaGroup b) {
CellRangeAddress8Bit rangeA = ((SharedValueGroup)a).getSVR().getRange(); CellRangeAddress8Bit rangeA = a.getSFR().getRange();
CellRangeAddress8Bit rangeB = ((SharedValueGroup)b).getSVR().getRange(); CellRangeAddress8Bit rangeB = b.getSFR().getRange();
int cmp; int cmp;
cmp = rangeA.getFirstRow() - rangeB.getFirstRow(); cmp = rangeA.getFirstRow() - rangeB.getFirstRow();
@ -228,44 +207,57 @@ public final class SharedValueManager {
}; };
/** /**
* The {@link SharedValueRecordBase} record returned by this method * Gets the {@link SharedValueRecordBase} record if it should be encoded immediately after the
* @param firstCell the cell coordinates as read from the {@link ExpPtg} or {@link TblPtg} * formula record contained in the specified {@link FormulaRecordAggregate} agg. Note - the
* of the current formula. Note - this is usually not the same as the cell coordinates * shared value record always appears after the first formula record in the group. For arrays
* of the formula's cell. * 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 * @return the SHRFMLA, TABLE or ARRAY record for the formula cell, if it is the first cell of
* table or array region. <code>null</code> if * a table or array region. <code>null</code> 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 row = firstCell.getRow();
int column = firstCell.getCol(); int column = firstCell.getCol();
boolean isTopLeft = agg.getRow() == row && agg.getColumn() == column; if (agg.getRow() != row || agg.getColumn() != column) {
if (isTopLeft) { // not the first formula cell in the group
for (int i = 0; i < _tableRecords.length; i++) { return null;
TableRecord tr = _tableRecords[i];
if (tr.isFirstCell(row, column)) {
return tr;
}
}
for (int i = 0; i < _arrayRecords.length; i++) {
ArrayRecord ar = _arrayRecords[i];
if (ar.isFirstCell(row, column)) {
return ar;
}
}
} 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(); SharedFormulaGroup[] groups = getGroups();
for (int i = 0; i < groups.length; i++) { for (int i = 0; i < groups.length; i++) {
SharedValueGroup svg = groups[i]; // note - logic for finding correct shared formula group is slightly
SharedValueRecordBase svr = svg.getSVR(); // more complicated since the first cell
if (svr.isFirstCell(row, column)) { SharedFormulaGroup sfg = groups[i];
if (svg.isFirstMember(agg)) { if (sfg.isFirstCell(row, column)) {
return svr; return sfg.getSFR();
} }
return null; }
// 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;
}
}
for (int i = 0; i < _arrayRecords.length; i++) {
ArrayRecord ar = _arrayRecords[i];
if (ar.isFirstCell(row, column)) {
return ar;
} }
} }
return null; return null;
@ -276,7 +268,7 @@ public final class SharedValueManager {
* to plain unshared formulas * to plain unshared formulas
*/ */
public void unlink(SharedFormulaRecord sharedFormulaRecord) { public void unlink(SharedFormulaRecord sharedFormulaRecord) {
SharedValueGroup svg = (SharedValueGroup) _groupsBySharedFormulaRecord.remove(sharedFormulaRecord); SharedFormulaGroup svg = _groupsBySharedFormulaRecord.remove(sharedFormulaRecord);
_groups = null; // be sure to reset cached value _groups = null; // be sure to reset cached value
if (svg == null) { if (svg == null) {
throw new IllegalStateException("Failed to find formulas for shared formula"); throw new IllegalStateException("Failed to find formulas for shared formula");

View File

@ -26,6 +26,7 @@ import junit.framework.TestCase;
import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.hssf.HSSFTestDataSamples;
import org.apache.poi.hssf.record.Record; import org.apache.poi.hssf.record.Record;
import org.apache.poi.hssf.record.SharedFormulaRecord; 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.HSSFSheet;
import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.hssf.usermodel.HSSFWorkbook;
import org.apache.poi.hssf.usermodel.RecordInspector; import org.apache.poi.hssf.usermodel.RecordInspector;
@ -120,4 +121,52 @@ public final class TestSharedValueManager extends TestCase {
} }
assertEquals(2, count); 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.<br/>
*
* 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);
}
} }

Binary file not shown.