Fixed HSSFCell/FormulaRecordAggregate to properly remove StringRecord when cached result type changes.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@736476 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java

@@ -147,7 +147,7 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel
 				 rv.visitRecord(sharedFormulaRecord);
 			 }
 		 }
-		 if (_stringRecord != null) {
+		 if (_formulaRecord.hasCachedResultString() && _stringRecord != null) {
 			 rv.visitRecord(_stringRecord);
 		 }
 	}
@@ -180,6 +180,10 @@ public final class FormulaRecordAggregate extends RecordAggregate implements Cel
 		_stringRecord = null;
 		_formulaRecord.setCachedResultErrorCode(errorCode);
 	}
+	public void setCachedDoubleResult(double value) {
+		_stringRecord = null;
+		_formulaRecord.setValue(value);
+	}
 
 	public Ptg[] getFormulaTokens() {
 		if (_sharedFormulaRecord == null) {

src/java/org/apache/poi/hssf/usermodel/HSSFCell.java

@@ -72,7 +72,6 @@ import org.apache.poi.ss.formula.FormulaType;
  * @author  Dan Sherman (dsherman at isisph.com)
  * @author  Brian Sanders (kestrel at burdell dot org) Active Cell support
  * @author  Yegor Kozlov cell comments support
- * @version 1.0-pre
  */
 public class HSSFCell implements Cell {
     /** Numeric Cell type (0) @see #setCellType(int) @see #getCellType() */
@@ -481,7 +480,7 @@ public class HSSFCell implements Cell {
                 (( NumberRecord ) record).setValue(value);
                 break;
             case CELL_TYPE_FORMULA:
-                ((FormulaRecordAggregate)record).getFormulaRecord().setValue(value);
+                ((FormulaRecordAggregate)record).setCachedDoubleResult(value);
                 break;
         }
     }
@@ -743,7 +742,6 @@ public class HSSFCell implements Cell {
      *        precalculated value, for booleans we'll set its value. For other types we
      *        will change the cell to a boolean cell and set its value.
      */
-
     public void setCellValue(boolean value) {
         int row=record.getRow();
         short col=record.getColumn();
@@ -756,7 +754,7 @@ public class HSSFCell implements Cell {
                 (( BoolErrRecord ) record).setValue(value);
                 break;
             case CELL_TYPE_FORMULA:
-                ((FormulaRecordAggregate)record).getFormulaRecord().setCachedResultBoolean(value);
+                ((FormulaRecordAggregate)record).setCachedBooleanResult(value);
                 break;
         }
     }
@@ -780,7 +778,7 @@ public class HSSFCell implements Cell {
                 (( BoolErrRecord ) record).setValue(errorCode);
                 break;
             case CELL_TYPE_FORMULA:
-                ((FormulaRecordAggregate)record).getFormulaRecord().setCachedResultErrorCode(errorCode);
+                ((FormulaRecordAggregate)record).setCachedErrorResult(errorCode);
                 break;
         }
     }

src/testcases/org/apache/poi/hssf/usermodel/TestHSSFCell.java

@@ -25,7 +25,12 @@ import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.model.Sheet;
+import org.apache.poi.hssf.record.DBCellRecord;
+import org.apache.poi.hssf.record.FormulaRecord;
+import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.StringRecord;
 import org.apache.poi.hssf.util.HSSFColor;
+import org.apache.poi.ss.usermodel.ErrorConstants;
 
 /**
  * Tests various functionality having to do with {@link HSSFCell}.  For instance support for
@@ -123,8 +128,7 @@ public final class TestHSSFCell extends TestCase {
 
 		assertEquals("Date from file using 1900 Date Windowing",
 				date.getTime(),
-                            sheet.getRow(0).getCell(0)
+				sheet.getRow(0).getCell(0).getDateCellValue().getTime());
-                               .getDateCellValue().getTime());
 		 
 		// now check a file with 1904 Date Windowing
 		workbook = openSample("1904DateWindowing.xls");
@@ -132,8 +136,7 @@ public final class TestHSSFCell extends TestCase {
 
 		assertEquals("Date from file using 1904 Date Windowing",
 				date.getTime(),
-                            sheet.getRow(0).getCell(0)
+				sheet.getRow(0).getCell(0).getDateCellValue().getTime());
-                               .getDateCellValue().getTime());
 	}
 
 	/**
@@ -306,7 +309,7 @@ public final class TestHSSFCell extends TestCase {
 		assertEquals(1, link2.getFirstColumn());
 	}
 	
-    /*tests the toString() method of HSSFCell*/
+	/**tests the toString() method of HSSFCell*/
 	public void testToString() {
 		HSSFWorkbook wb = new HSSFWorkbook();
 		HSSFRow r = wb.createSheet("Sheet1").createRow(0);
@@ -361,11 +364,15 @@ public final class TestHSSFCell extends TestCase {
 		try {
 			styA.verifyBelongsToWorkbook(wbB);
 			fail();
-		} catch (IllegalArgumentException e) {}
+		} catch (IllegalArgumentException e) {
+			// expected during successful test
+		}
 		try {
 			styB.verifyBelongsToWorkbook(wbA);
 			fail();
-		} catch (IllegalArgumentException e) {}
+		} catch (IllegalArgumentException e) {
+			// expected during successful test
+		}
 
 		HSSFCell cellA = wbA.createSheet().createRow(0).createCell(0);
 		HSSFCell cellB = wbB.createSheet().createRow(0).createCell(0);
@@ -375,11 +382,15 @@ public final class TestHSSFCell extends TestCase {
 		try {
 			cellA.setCellStyle(styB);
 			fail();
-		} catch (IllegalArgumentException e) {}
+		} catch (IllegalArgumentException e) {
+			// expected during successful test
+		}
 		try {
 			cellB.setCellStyle(styA);
 			fail();
-		} catch (IllegalArgumentException e) {}
+		} catch (IllegalArgumentException e) {
+			// expected during successful test
+		}
 	}
 
 	public void testChangeTypeStringToBool() {
@@ -463,5 +474,52 @@ public final class TestHSSFCell extends TestCase {
 		}
 		assertEquals(true, cell.getBooleanCellValue());
 	}
+	
+	/**
+	 * Test for small bug observable around r736460 (prior to version 3.5).  POI fails to remove
+	 * the {@link StringRecord} following the {@link FormulaRecord} after the result type had been 
+	 * changed to number/boolean/error.  Excel silently ignores the extra record, but some POI
+	 * versions (prior to bug 46213 / r717883) crash instead.
+	 */
+	public void testCachedTypeChange() {
+		HSSFSheet sheet = new HSSFWorkbook().createSheet("Sheet1");
+		HSSFCell cell = sheet.createRow(0).createCell(0);
+		cell.setCellFormula("A1");
+		cell.setCellValue("abc");
+		confirmStringRecord(sheet, true);
+		cell.setCellValue(123);
+		Record[] recs = RecordInspector.getRecords(sheet, 0);
+		if (recs.length == 28 && recs[23] instanceof StringRecord) {
+			throw new AssertionFailedError("Identified bug - leftover StringRecord");
+		}
+		confirmStringRecord(sheet, false);
+		
+		// string to error code
+		cell.setCellValue("abc");
+		confirmStringRecord(sheet, true);
+		cell.setCellErrorValue((byte)ErrorConstants.ERROR_REF);
+		confirmStringRecord(sheet, false);
+		
+		// string to boolean
+		cell.setCellValue("abc");
+		confirmStringRecord(sheet, true);
+		cell.setCellValue(false);
+		confirmStringRecord(sheet, false);
+	}
+
+	private static void confirmStringRecord(HSSFSheet sheet, boolean isPresent) {
+		Record[] recs = RecordInspector.getRecords(sheet, 0);
+		assertEquals(isPresent ? 28 : 27, recs.length);
+		int index = 22;
+		Record fr = recs[index++];
+		assertEquals(FormulaRecord.class, fr.getClass());
+		if (isPresent) {
+			assertEquals(StringRecord.class, recs[index++].getClass());
+		} else {
+			assertFalse(StringRecord.class == recs[index].getClass());
+		}
+		Record dbcr = recs[index++];
+		assertEquals(DBCellRecord.class, dbcr.getClass());
+	}
 }