poi-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject 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/
Date Fri, 18 Sep 2009 00:33:19 GMT
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


Mime
View raw message