You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by jo...@apache.org on 2009/09/18 02:33:19 UTC
svn commit: r816417 - in /poi/trunk: src/documentation/content/xdocs/
src/java/org/apache/poi/hssf/model/
src/java/org/apache/poi/hssf/record/aggregates/
src/testcases/org/apache/poi/hssf/record/aggregates/ test-data/spreadsheet/
Author: josh
Date: Fri Sep 18 00:33:18 2009
New Revision: 816417
URL: http://svn.apache.org/viewvc?rev=816417&view=rev
Log:
Bugzilla 47747 - fixed logic for locating shared formula records
Added:
poi/trunk/test-data/spreadsheet/ex47747-sharedFormula.xls (with props)
Modified:
poi/trunk/src/documentation/content/xdocs/status.xml
poi/trunk/src/java/org/apache/poi/hssf/model/RowBlocksReader.java
poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java
poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java
poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestSharedValueManager.java
Modified: poi/trunk/src/documentation/content/xdocs/status.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/status.xml?rev=816417&r1=816416&r2=816417&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/status.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/status.xml Fri Sep 18 00:33:18 2009
@@ -33,8 +33,9 @@
<changes>
<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="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">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>
Modified: poi/trunk/src/java/org/apache/poi/hssf/model/RowBlocksReader.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/model/RowBlocksReader.java?rev=816417&r1=816416&r2=816417&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/model/RowBlocksReader.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/model/RowBlocksReader.java Fri Sep 18 00:33:18 2009
@@ -21,17 +21,19 @@
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 RowBlocksReader(RecordStream rs) {
List<Record> plainRecords = new ArrayList<Record>();
List<Record> shFrmRecords = new ArrayList<Record>();
+ List<CellReference> firstCellRefs = new ArrayList<CellReference>();
List<Record> arrayRecords = new ArrayList<Record>();
List<Record> tableRecords = new ArrayList<Record>();
List<Record> mergeCellRecords = new ArrayList<Record>();
+ 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<Record> 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 <tt>MergeCellsRecord</tt>s found
*/
@@ -97,7 +110,7 @@
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() {
Modified: poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java?rev=816417&r1=816416&r2=816417&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java Fri Sep 18 00:33:18 2009
@@ -58,10 +58,12 @@
} 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 @@
_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.<br/>
- *
+ *
* 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()).<p/>
*/
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 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);
Modified: poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java?rev=816417&r1=816416&r2=816417&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java Fri Sep 18 00:33:18 2009
@@ -28,9 +28,8 @@
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 @@
*/
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 <i>but not always</i> 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 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 <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() {
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 <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(
- 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<SharedFormulaRecord, SharedFormulaGroup> _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<SharedFormulaRecord, SharedFormulaGroup> m = new HashMap<SharedFormulaRecord, SharedFormulaGroup>(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 @@
* 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 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 <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);
+ // 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 @@
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) {
- 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 @@
};
/**
- * 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. <code>null</code> if
+ * @return the SHRFMLA, TABLE or ARRAY record for the formula cell, if it is the first cell of
+ * 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 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 @@
* 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");
Modified: poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestSharedValueManager.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestSharedValueManager.java?rev=816417&r1=816416&r2=816417&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestSharedValueManager.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestSharedValueManager.java Fri Sep 18 00:33:18 2009
@@ -26,6 +26,7 @@
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 @@
}
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);
+ }
}
Added: poi/trunk/test-data/spreadsheet/ex47747-sharedFormula.xls
URL: http://svn.apache.org/viewvc/poi/trunk/test-data/spreadsheet/ex47747-sharedFormula.xls?rev=816417&view=auto
==============================================================================
Binary file - no diff available.
Propchange: poi/trunk/test-data/spreadsheet/ex47747-sharedFormula.xls
------------------------------------------------------------------------------
svn:executable = *
Propchange: poi/trunk/test-data/spreadsheet/ex47747-sharedFormula.xls
------------------------------------------------------------------------------
svn:mime-type = application/octet-stream
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org