Return-Path: 
 <commits-return-1341-apmail-poi-commits-archive=poi.apache.org@poi.apache.org>
Delivered-To: apmail-poi-commits-archive@locus.apache.org
Received: (qmail 23922 invoked from network); 7 Nov 2008 23:17:10 -0000
Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2)
  by minotaur.apache.org with SMTP; 7 Nov 2008 23:17:10 -0000
Received: (qmail 12418 invoked by uid 500); 7 Nov 2008 23:17:17 -0000
Delivered-To: apmail-poi-commits-archive@poi.apache.org
Received: (qmail 12381 invoked by uid 500); 7 Nov 2008 23:17:17 -0000
Mailing-List: contact commits-help@poi.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:commits-help@poi.apache.org>
List-Unsubscribe: <mailto:commits-unsubscribe@poi.apache.org>
List-Post: <mailto:commits@poi.apache.org>
List-Id: <commits.poi.apache.org>
Reply-To: dev@poi.apache.org
Delivered-To: mailing list commits@poi.apache.org
Received: (qmail 12372 invoked by uid 99); 7 Nov 2008 23:17:16 -0000
Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136)
    by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 07 Nov 2008 15:17:16 -0800
X-ASF-Spam-Status: No, hits=-2000.0 required=10.0
	tests=ALL_TRUSTED
X-Spam-Check-By: apache.org
Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4)
    by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 07 Nov 2008 23:16:06 +0000
Received: by eris.apache.org (Postfix, from userid 65534)
	id 13DE3238889D; Fri,  7 Nov 2008 15:16:49 -0800 (PST)
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r712307 - in /poi/trunk/src: documentation/content/xdocs/
 java/org/apache/poi/hssf/record/
 java/org/apache/poi/hssf/record/aggregates/ java/org/apache/poi/ss/formula/
 java/org/apache/poi/ss/util/ testcases/org/apache/poi/hssf/data/ testcase...
Date: Fri, 07 Nov 2008 23:16:48 -0000
To: commits@poi.apache.org
From: josh@apache.org
X-Mailer: svnmailer-1.0.8
Message-Id: <20081107231649.13DE3238889D@eris.apache.org>
X-Virus-Checked: Checked by ClamAV on apache.org

Author: josh
Date: Fri Nov  7 15:16:48 2008
New Revision: 712307

URL: http://svn.apache.org/viewvc?rev=712307&view=rev
Log:
Fixed problem with linking shared formulas when ranges overlap

Added:
    poi/trunk/src/testcases/org/apache/poi/hssf/data/overlapSharedFormula.xls   (with props)
    poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestSharedValueManager.java
Modified:
    poi/trunk/src/documentation/content/xdocs/changes.xml
    poi/trunk/src/documentation/content/xdocs/status.xml
    poi/trunk/src/java/org/apache/poi/hssf/record/FormulaRecord.java
    poi/trunk/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.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/java/org/apache/poi/ss/formula/Formula.java
    poi/trunk/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java
    poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/AllRecordAggregateTests.java
    poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java

Modified: poi/trunk/src/documentation/content/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/changes.xml?rev=712307&r1=712306&r2=712307&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/changes.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/changes.xml Fri Nov  7 15:16:48 2008
@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.5-beta4" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">Fixed problem with linking shared formulas when ranges overlap</action>
            <action dev="POI-DEVELOPERS" type="fix">45784 - More fixes to SeriesTextRecord</action>
            <action dev="POI-DEVELOPERS" type="fix">46033 - fixed TableCell to correctly set text type</action>
            <action dev="POI-DEVELOPERS" type="fix">46122 - fixed Picture.draw to skip rendering if picture data was not found</action>

Modified: poi/trunk/src/documentation/content/xdocs/status.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/status.xml?rev=712307&r1=712306&r2=712307&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/status.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/status.xml Fri Nov  7 15:16:48 2008
@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.5-beta4" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">Fixed problem with linking shared formulas when ranges overlap</action>
            <action dev="POI-DEVELOPERS" type="fix">45784 - More fixes to SeriesTextRecord</action>
            <action dev="POI-DEVELOPERS" type="fix">46033 - fixed TableCell to correctly set text type</action>
            <action dev="POI-DEVELOPERS" type="fix">46122 - fixed Picture.draw to skip rendering if picture data was not found</action>

Modified: poi/trunk/src/java/org/apache/poi/hssf/record/FormulaRecord.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/FormulaRecord.java?rev=712307&r1=712306&r2=712307&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/record/FormulaRecord.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/record/FormulaRecord.java Fri Nov  7 15:16:48 2008
@@ -345,6 +345,10 @@
 		return field_8_parsed_expr.getTokens();
 	}
 
+	public Formula getFormula() {
+		return field_8_parsed_expr;
+	}
+
 	public void setParsedExpression(Ptg[] ptgs) {
 		field_8_parsed_expr = Formula.create(ptgs);
 	}

Modified: poi/trunk/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java?rev=712307&r1=712306&r2=712307&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java Fri Nov  7 15:16:48 2008
@@ -182,4 +182,7 @@
         result.field_7_parsed_expr = field_7_parsed_expr.copy();
         return result;
     }
+	public boolean isFormulaSame(SharedFormulaRecord other) {
+		return field_7_parsed_expr.isSame(other.field_7_parsed_expr);
+	}
 }

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=712307&r1=712306&r2=712307&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 Nov  7 15:16:48 2008
@@ -25,6 +25,8 @@
 import org.apache.poi.hssf.record.StringRecord;
 import org.apache.poi.hssf.record.formula.ExpPtg;
 import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.hssf.util.CellReference;
+import org.apache.poi.ss.formula.Formula;
 
 /**
  * The formula record aggregate is used to join together the formula record and it's
@@ -57,15 +59,19 @@
 					+ (hasCachedStringFlag ? "" : "not ") + " set");
 		}
 
-		_formulaRecord = formulaRec;
-		_sharedValueManager = svm;
-		_stringRecord = stringRec;
 		if (formulaRec.isSharedFormula()) {
-			_sharedFormulaRecord = svm.linkSharedFormulaRecord(this);
-			if (_sharedFormulaRecord == null) {
+			CellReference cr = new CellReference(formulaRec.getRow(), formulaRec.getColumn());
+			
+			CellReference firstCell = formulaRec.getFormula().getExpReference();
+			if (firstCell == null) {
 				handleMissingSharedFormulaRecord(formulaRec);
+			} else {
+				_sharedFormulaRecord = svm.linkSharedFormulaRecord(firstCell, this);
 			}
 		}
+		_formulaRecord = formulaRec;
+		_sharedValueManager = svm;
+		_stringRecord = stringRec;
 	}
 	/**
 	 * Sometimes the shared formula flag "seems" to be erroneously set (because the corresponding
@@ -132,10 +138,14 @@
 
 	public void visitContainedRecords(RecordVisitor rv) {
 		 rv.visitRecord(_formulaRecord);
-		 // TODO - only bother with this if array or table formula
-		 Record sharedFormulaRecord = _sharedValueManager.getRecordForFirstCell(_formulaRecord);
-		 if (sharedFormulaRecord != null) {
-			 rv.visitRecord(sharedFormulaRecord);
+		 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);
+			 }
 		 }
 		 if (_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=712307&r1=712306&r2=712307&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 Nov  7 15:16:48 2008
@@ -17,6 +17,8 @@
 
 package org.apache.poi.hssf.record.aggregates;
 
+import java.util.Arrays;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -25,6 +27,10 @@
 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;
 
 /**
  * Manages various auxiliary records while constructing a
@@ -34,15 +40,15 @@
  * <li>{@link ArrayRecord}s</li>
  * <li>{@link TableRecord}s</li>
  * </ul>
- * 
+ *
  * @author Josh Micich
  */
 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 final FormulaRecordAggregate[] _frAggs;
+		private FormulaRecordAggregate[] _frAggs;
 		private int _numberOfFormulas;
 
 		public SharedValueGroup(SharedValueRecordBase svr) {
@@ -54,6 +60,12 @@
 		}
 
 		public void add(FormulaRecordAggregate agg) {
+			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;
+			}
 			_frAggs[_numberOfFormulas++] = agg;
 		}
 
@@ -63,10 +75,6 @@
 			}
 		}
 
-		public boolean isInRange(int rowIx, int columnIx) {
-			return _svr.isInRange(rowIx, columnIx);
-		}
-
 		public SharedValueRecordBase getSVR() {
 			return _svr;
 		}
@@ -77,13 +85,16 @@
 		 * 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 isFirstCell(int row, int column) {
-			// hack for the moment, just check against the first formula that 
-			// came in through the add() method.
-			FormulaRecordAggregate fra = _frAggs[0];
-			return fra.getRow() == row && fra.getColumn() == column;
+		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("]");
+			return sb.toString();
 		}
-		
 	}
 
 	public static final SharedValueManager EMPTY = new SharedValueManager(
@@ -106,6 +117,13 @@
 		_groupsBySharedFormulaRecord = m;
 	}
 
+	/**
+	 * @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
+	 * that this code does not inadvertently collect <tt>SharedFormulaRecord</tt>s from any other
+	 * 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) {
@@ -116,77 +134,145 @@
 
 
 	/**
-	 * @return <code>null</code> if the specified formula does not have any corresponding
-	 * {@link SharedFormulaRecord}
+	 * @param firstCell as extracted from the {@link ExpPtg} from the cell's formula.
+	 * @return never <code>null</code>
 	 */
-	public SharedFormulaRecord linkSharedFormulaRecord(FormulaRecordAggregate agg) {
-		FormulaRecord formula = agg.getFormulaRecord();
-		int row = formula.getRow();
-		int column = formula.getColumn();
-		// Traverse the list of shared formulas in
-		// reverse order, and try to find the correct one
-		// for us
-		
-		SharedValueGroup[] groups = getGroups();
+	public SharedFormulaRecord linkSharedFormulaRecord(CellReference firstCell, FormulaRecordAggregate agg) {
+
+		SharedValueGroup result = findGroup(getGroups(), firstCell);
+		result.add(agg);
+		return (SharedFormulaRecord) result.getSVR();
+	}
+
+	private static SharedValueGroup findGroup(SharedValueGroup[] 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 svr = groups[i];
-			if (svr.isInRange(row, column)) {
-				svr.add(agg);
-				return (SharedFormulaRecord) svr.getSVR();
+			SharedValueGroup svg = groups[i];
+			if (svg.getSVR().isFirstCell(row, column)) {
+				return svg;
 			}
 		}
-		return null;
+		// 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);
 	}
 
 	private SharedValueGroup[] getGroups() {
 		if (_groups == null) {
 			SharedValueGroup[] groups = new SharedValueGroup[_groupsBySharedFormulaRecord.size()];
 			_groupsBySharedFormulaRecord.values().toArray(groups);
+			Arrays.sort(groups, SVGComparator); // make search behaviour more deterministic
 			_groups = groups;
-			
 		}
 		return _groups;
 	}
 
+	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();
+
+			int cmp;
+			cmp = rangeA.getFirstRow() - rangeB.getFirstRow();
+			if (cmp != 0) {
+				return cmp;
+			}
+			cmp = rangeA.getFirstColumn() - rangeB.getFirstColumn();
+			if (cmp != 0) {
+				return cmp;
+			}
+			return 0;
+		}
+	};
 
 	/**
-	 * Note - does not return SharedFormulaRecords currently, because the corresponding formula
-	 * records have been converted to 'unshared'. POI does not attempt to re-share formulas. On
-	 * the other hand, there is no such conversion for array or table formulas, so this method 
-	 * returns the TABLE or ARRAY record (if it should be written after the specified 
-	 * formulaRecord.
-	 * 
-	 * @return the TABLE or ARRAY record for this formula cell, if it is the first cell of a 
-	 * table or array region.
+	 * 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.
+	 *
+	 * @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
 	 */
-	public SharedValueRecordBase getRecordForFirstCell(FormulaRecord formulaRecord) {
-		int row = formulaRecord.getRow();
-		int column = formulaRecord.getColumn();
-		for (int i = 0; i < _tableRecords.length; i++) {
-			TableRecord tr = _tableRecords[i];
-			if (tr.isFirstCell(row, column)) {
-				return tr;
+	public SharedValueRecordBase getRecordForFirstCell(CellReference firstCell, FormulaRecordAggregate agg) {
+		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;
+				}
 			}
-		}
-		for (int i = 0; i < _arrayRecords.length; i++) {
-			ArrayRecord ar = _arrayRecords[i];
-			if (ar.isFirstCell(row, column)) {
-				return ar;
+			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();
 		for (int i = 0; i < groups.length; i++) {
 			SharedValueGroup svg = groups[i];
-			if (svg.isFirstCell(row, column)) {
-				return svg.getSVR();
+			SharedValueRecordBase svr = svg.getSVR();
+			if (svr.isFirstCell(row, column)) {
+				if (svg.isFirstMember(agg)) {
+					return svr;
+				}
+				return null;
 			}
 		}
 		return null;
 	}
 
 	/**
-	 * Converts all {@link FormulaRecord}s handled by <tt>sharedFormulaRecord</tt> 
+	 * Converts all {@link FormulaRecord}s handled by <tt>sharedFormulaRecord</tt>
 	 * to plain unshared formulas
 	 */
 	public void unlink(SharedFormulaRecord sharedFormulaRecord) {

Modified: poi/trunk/src/java/org/apache/poi/ss/formula/Formula.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/Formula.java?rev=712307&r1=712306&r2=712307&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/Formula.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/Formula.java Fri Nov  7 15:16:48 2008
@@ -1,6 +1,15 @@
 package org.apache.poi.ss.formula;
 
+import java.util.Arrays;
+
+import org.apache.poi.hssf.record.ArrayRecord;
+import org.apache.poi.hssf.record.SharedFormulaRecord;
+import org.apache.poi.hssf.record.TableRecord;
+import org.apache.poi.hssf.record.formula.ExpPtg;
 import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.hssf.record.formula.TblPtg;
+import org.apache.poi.hssf.util.CellReference;
+import org.apache.poi.util.LittleEndian;
 import org.apache.poi.util.LittleEndianByteArrayInputStream;
 import org.apache.poi.util.LittleEndianInput;
 import org.apache.poi.util.LittleEndianOutput;
@@ -130,4 +139,35 @@
 		// OK to return this for the moment because currently immutable
 		return this;
 	}
+	
+	/**
+	 * Gets the locator for the corresponding {@link SharedFormulaRecord}, {@link ArrayRecord} or
+	 * {@link TableRecord} if this formula belongs to such a grouping.  The {@link CellReference}
+	 * returned by this method will  match the top left corner of the range of that grouping. 
+	 * The return value is usually not the same as the location of the cell containing this formula.
+	 * 
+	 * @return the firstRow & firstColumn of an array formula or shared formula that this formula
+	 * belongs to.  <code>null</code> if this formula is not part of an array or shared formula.
+	 */
+	public CellReference getExpReference() {
+		byte[] data = _byteEncoding;
+		if (data.length != 5) {
+			// tExp and tTbl are always 5 bytes long, and the only ptg in the formula
+			return null;
+		}
+		switch (data[0]) {
+			case ExpPtg.sid:
+				break;
+			case TblPtg.sid:
+				break;
+			default:
+				return null;
+		}
+		int firstRow = LittleEndian.getUShort(data, 1);
+		int firstColumn = LittleEndian.getUShort(data, 3);
+		return new CellReference(firstRow, firstColumn);
+	}
+	public boolean isSame(Formula other) {
+		return Arrays.equals(_byteEncoding, other._byteEncoding);
+	}
 }

Modified: poi/trunk/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java?rev=712307&r1=712306&r2=712307&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/util/CellRangeAddressBase.java Fri Nov  7 15:16:48 2008
@@ -46,8 +46,7 @@
 		_firstCol = firstCol;
 		_lastCol = lastCol;
 	}
-	private static boolean isValid(int firstRow, int lastRow, int firstColumn, int lastColumn)
-	{
+	private static boolean isValid(int firstRow, int lastRow, int firstColumn, int lastColumn) {
 		if(lastRow < 0 || lastRow > LAST_ROW_INDEX) {
 			return false;
 		}
@@ -129,6 +128,8 @@
 	}
 
 	public final String toString() {
-		return getClass().getName() + " ["+_firstRow+", "+_lastRow+", "+_firstCol+", "+_lastCol+"]";
+		CellReference crA = new CellReference(_firstRow, _firstCol);
+		CellReference crB = new CellReference(_lastRow, _lastCol);
+		return getClass().getName() + " [" + crA.formatAsString() + ":" + crB.formatAsString() +"]";
 	}
 }

Added: poi/trunk/src/testcases/org/apache/poi/hssf/data/overlapSharedFormula.xls
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/data/overlapSharedFormula.xls?rev=712307&view=auto
==============================================================================
Binary file - no diff available.

Propchange: poi/trunk/src/testcases/org/apache/poi/hssf/data/overlapSharedFormula.xls
------------------------------------------------------------------------------
    svn:mime-type = application/octet-stream

Modified: poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/AllRecordAggregateTests.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/AllRecordAggregateTests.java?rev=712307&r1=712306&r2=712307&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/AllRecordAggregateTests.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/AllRecordAggregateTests.java Fri Nov  7 15:16:48 2008
@@ -34,6 +34,7 @@
 		result.addTestSuite(TestColumnInfoRecordsAggregate.class);
 		result.addTestSuite(TestFormulaRecordAggregate.class);
 		result.addTestSuite(TestRowRecordsAggregate.class);
+		result.addTestSuite(TestSharedValueManager.class);
 		result.addTestSuite(TestValueRecordsAggregate.class);
 		return result;
 	}

Added: 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=712307&view=auto
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestSharedValueManager.java (added)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/record/aggregates/TestSharedValueManager.java Fri Nov  7 15:16:48 2008
@@ -0,0 +1,126 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.hssf.record.aggregates;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Collection;
+import java.util.HashMap;
+
+import junit.framework.AssertionFailedError;
+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.HSSFSheet;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+import org.apache.poi.hssf.usermodel.RecordInspector;
+
+/**
+ * Tests for {@link SharedValueManager}
+ * 
+ * @author Josh Micich
+ */
+public final class TestSharedValueManager extends TestCase {
+
+	/**
+	 * This Excel workbook contains two sheets that each have a pair of overlapping shared formula
+	 * ranges.  The first sheet has one row and one column shared formula ranges which intersect.
+	 * The second sheet has two column shared formula ranges - one contained within the other.
+	 * These shared formula ranges were created by fill-dragging a single cell formula across the
+	 * desired region.  The larger shared formula ranges were placed first.<br/>
+	 * 
+	 * There are probably many ways to produce similar effects, but it should be noted that Excel
+	 * is quite temperamental in this regard.  Slight variations in technique can cause the shared
+	 * formulas to spill out into plain formula records (which would make these tests pointless).
+	 * 
+	 */
+	private static final String SAMPLE_FILE_NAME = "overlapSharedFormula.xls";
+	/**
+	 * Some of these bugs are intermittent, and the test author couldn't think of a way to write 
+	 * test code to hit them bug deterministically. The reason for the unpredictability is that
+	 * the bugs depended on the {@link SharedFormulaRecord}s being searched in a particular order.
+	 * At the time of writing of the test, the order was being determined by the call to {@link 
+	 * Collection#toArray(Object[])} on {@link HashMap#values()} where the items in the map were 
+	 * using default {@link Object#hashCode()}<br/>
+	 */
+	private static final int MAX_ATTEMPTS=5;
+
+	/**
+	 * This bug happened when there were two or more shared formula ranges that overlapped.  POI
+	 * would sometimes associate formulas in the overlapping region with the wrong shared formula
+	 */
+	public void testPartiallyOverlappingRanges() throws IOException {
+		Record[] records;
+
+		int attempt=1;
+		do {
+    		HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook(SAMPLE_FILE_NAME);
+    		
+    		HSSFSheet sheet = wb.getSheetAt(0);
+    		RecordInspector.getRecords(sheet, 0);
+    		assertEquals("1+1", sheet.getRow(2).getCell(0).getCellFormula());
+    		if ("1+1".equals(sheet.getRow(3).getCell(0).getCellFormula())) {
+    			throw new AssertionFailedError("Identified bug - wrong shared formula record chosen"
+    					+ " (attempt " + attempt + ")");
+    		}
+    		assertEquals("2+2", sheet.getRow(3).getCell(0).getCellFormula());
+    		records = RecordInspector.getRecords(sheet, 0);
+		} while (attempt++ < MAX_ATTEMPTS);
+    		
+		int count=0;
+		for (int i = 0; i < records.length; i++) {
+			if (records[i] instanceof SharedFormulaRecord) {
+				count++;
+			}
+		}
+		assertEquals(2, count);
+	}
+	
+	/**
+	 * This bug occurs for similar reasons to the bug in {@link #testPartiallyOverlappingRanges()}
+	 * but the symptoms are much uglier - serialization fails with {@link NullPointerException}.<br/>
+	 */
+	public void testCompletelyOverlappedRanges() throws IOException {
+		Record[] records;
+
+		int attempt=1;
+		do {
+    		HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook(SAMPLE_FILE_NAME);
+    			
+    		HSSFSheet sheet = wb.getSheetAt(1);
+    		try {
+    			records = RecordInspector.getRecords(sheet, 0);
+    		} catch (NullPointerException e) {
+    			throw new AssertionFailedError("Identified bug " +
+    					"- cannot reserialize completely overlapped shared formula"
+    					+ " (attempt " + attempt + ")");
+    		}
+		} while (attempt++ < MAX_ATTEMPTS);
+		
+		int count=0;
+		for (int i = 0; i < records.length; i++) {
+			if (records[i] instanceof SharedFormulaRecord) {
+				count++;
+			}
+		}
+		assertEquals(2, count);
+	}
+}

Modified: poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java?rev=712307&r1=712306&r2=712307&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java Fri Nov  7 15:16:48 2008
@@ -60,6 +60,7 @@
         if (true) { // set to false to output test files
             return;
         }
+        System.setProperty("poi.keep.tmp.files", "true");
         File file;
         try {
             file = TempFile.createTempFile(simpleFileName + "#", ".xls");



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org