Fix for bug 46479 - conversion of cached formula values and HSSFFormulaEvaluator.evaluateInCell()

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

src/documentation/content/xdocs/changes.xml

@@ -37,11 +37,12 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.5-beta5" date="2008-??-??">
-           <action dev="POI-DEVELOPERS" type="add">45031- added implementation for CHOOSE() function</action>
+           <action dev="POI-DEVELOPERS" type="fix">46479 - fixed bugs related to cached formula values and HSSFFormulaEvaluator.evaluateInCell()</action>
+           <action dev="POI-DEVELOPERS" type="add">45031 - added implementation for CHOOSE() function</action>
            <action dev="POI-DEVELOPERS" type="fix">46361 - resolve licensing issues around the HDGF resource file, chunks_parse_cmds.tbl</action>
            <action dev="POI-DEVELOPERS" type="add">46410 - added implementation for TIME() function</action>
            <action dev="POI-DEVELOPERS" type="add">46320 - added HSSFPictureData.getFormat()</action>
-           <action dev="POI-DEVELOPERS" type="fix">46445 fixed HSSFSheet.shiftRow to move hyperlinks</action>
+           <action dev="POI-DEVELOPERS" type="fix">46445 - fixed HSSFSheet.shiftRow to move hyperlinks</action>
            <action dev="POI-DEVELOPERS" type="fix">fixed formula parser to correctly resolve sheet-level names</action>
            <action dev="POI-DEVELOPERS" type="fix">46433 - support for shared formulas in XSSF</action>
            <action dev="POI-DEVELOPERS" type="add">46299 - support for carriage return and line break in XWPFRun</action>

src/documentation/content/xdocs/status.xml

@@ -34,11 +34,12 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.5-beta5" date="2008-??-??">
-           <action dev="POI-DEVELOPERS" type="add">45031- added implementation for CHOOSE() function</action>
+           <action dev="POI-DEVELOPERS" type="fix">46479 - fixed bugs related to cached formula values and HSSFFormulaEvaluator.evaluateInCell()</action>
+           <action dev="POI-DEVELOPERS" type="add">45031 - added implementation for CHOOSE() function</action>
            <action dev="POI-DEVELOPERS" type="fix">46361 - resolve licensing issues around the HDGF resource file, chunks_parse_cmds.tbl</action>
            <action dev="POI-DEVELOPERS" type="add">46410 - added implementation for TIME() function</action>
            <action dev="POI-DEVELOPERS" type="add">46320 - added HSSFPictureData.getFormat()</action>
-           <action dev="POI-DEVELOPERS" type="fix">46445 fixed HSSFSheet.shiftRow to move hyperlinks</action>
+           <action dev="POI-DEVELOPERS" type="fix">46445 - fixed HSSFSheet.shiftRow to move hyperlinks</action>
            <action dev="POI-DEVELOPERS" type="fix">fixed formula parser to correctly resolve sheet-level names</action>
            <action dev="POI-DEVELOPERS" type="fix">46433 - support for shared formulas in XSSF</action>
            <action dev="POI-DEVELOPERS" type="add">46299 - support for carriage return and line break in XWPFRun</action>

src/java/org/apache/poi/hssf/record/FormulaRecord.java

@@ -122,7 +122,7 @@ public final class FormulaRecord extends StandardRecord implements CellValueReco
 			return create(STRING, 0);
 		}
 		public static SpecialCachedValue createCachedBoolean(boolean b) {
-			return create(BOOLEAN, b ? 0 : 1);
+			return create(BOOLEAN, b ? 1 : 0);
 		}
 		public static SpecialCachedValue createCachedErrorCode(int errorCode) {
 			return create(ERROR_CODE, errorCode);

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

@@ -470,7 +470,7 @@ public class HSSFCell implements Cell {
         switch (cellType) {
             default:
                 setCellType(CELL_TYPE_NUMERIC, false, row, col, styleIndex);
-            case CELL_TYPE_ERROR:
+            case CELL_TYPE_NUMERIC:
                 (( NumberRecord ) record).setValue(value);
                 break;
             case CELL_TYPE_FORMULA:
@@ -745,7 +745,7 @@ public class HSSFCell implements Cell {
         switch (cellType) {
             default:
                 setCellType(CELL_TYPE_BOOLEAN, false, row, col, styleIndex);
-            case CELL_TYPE_ERROR:
+            case CELL_TYPE_BOOLEAN:
                 (( BoolErrRecord ) record).setValue(value);
                 break;
             case CELL_TYPE_FORMULA:
@@ -797,10 +797,13 @@ public class HSSFCell implements Cell {
             case CELL_TYPE_NUMERIC:
                 return ((NumberRecord)record).getValue() != 0;
 
-            // All other cases convert to false
-            // These choices are not well justified.
             case CELL_TYPE_FORMULA:
-                // should really evaluate, but HSSFCell can't call HSSFFormulaEvaluator
+                // use cached formula result if it's the right type:
+                FormulaRecord fr = ((FormulaRecordAggregate)record).getFormulaRecord();
+                checkFormulaCachedValueType(CELL_TYPE_BOOLEAN, fr);
+                return fr.getCachedBooleanValue();
+            // Other cases convert to false
+            // These choices are not well justified.
             case CELL_TYPE_ERROR:
             case CELL_TYPE_BLANK:
                 return false;

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

@@ -215,8 +215,8 @@ public class HSSFFormulaEvaluator implements FormulaEvaluator  {
 		HSSFCell result = (HSSFCell) cell;
 		if (cell.getCellType() == HSSFCell.CELL_TYPE_FORMULA) {
 			CellValue cv = evaluateFormulaCellValue(cell);
-			setCellType(cell, cv); // cell will no longer be a formula cell
 			setCellValue(cell, cv);
+			setCellType(cell, cv); // cell will no longer be a formula cell
 		}
 		return result;
 	}

src/testcases/org/apache/poi/hssf/record/TestFormulaRecord.java

@@ -17,6 +17,7 @@
 
 package org.apache.poi.hssf.record;
 
+import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.record.formula.AttrPtg;
@@ -28,8 +29,7 @@ import org.apache.poi.hssf.usermodel.HSSFCell;
 import org.apache.poi.hssf.usermodel.HSSFErrorConstants;
 
 /**
- * Tests the serialization and deserialization of the FormulaRecord
+ * Tests for {@link FormulaRecord}
- * class works correctly.
  *
  * @author Andrew C. Oliver
  */
@@ -168,4 +168,26 @@ public final class TestFormulaRecord extends TestCase {
 		RefPtg rp = (RefPtg) ptgs[0];
 		assertEquals("B$5", rp.toFormulaString());
 	}
+	
+	/**
+	 * Bug noticed while fixing 46479.  Operands of conditional operator ( ? : ) were swapped
+	 * inside {@link FormulaRecord}
+	 */
+	public void testCachedValue_bug46479() {
+		FormulaRecord fr0 = new FormulaRecord();
+		FormulaRecord fr1 = new FormulaRecord();
+		// test some other cached value types 
+		fr0.setValue(3.5);
+		assertEquals(3.5, fr0.getValue(), 0.0);
+		fr0.setCachedResultErrorCode(HSSFErrorConstants.ERROR_REF);
+		assertEquals(HSSFErrorConstants.ERROR_REF, fr0.getCachedErrorValue());
+
+		fr0.setCachedResultBoolean(false);
+		fr1.setCachedResultBoolean(true);
+		if (fr0.getCachedBooleanValue() == true && fr1.getCachedBooleanValue() == false) {
+			throw new AssertionFailedError("Identified bug 46479c");
+		}
+		assertEquals(false, fr0.getCachedBooleanValue());
+		assertEquals(true, fr1.getCachedBooleanValue());
+	}
 }

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

@@ -422,5 +422,46 @@ public final class TestHSSFCell extends TestCase {
 		}
 		assertEquals("TRUE", cell.getRichStringCellValue().getString());
 	}
+
+	public void testChangeTypeErrorToNumber_bug46479() {
+		HSSFCell cell = new HSSFWorkbook().createSheet("Sheet1").createRow(0).createCell(0);
+		cell.setCellErrorValue((byte)HSSFErrorConstants.ERROR_NAME);
+		try {
+			cell.setCellValue(2.5);
+		} catch (ClassCastException e) {
+			throw new AssertionFailedError("Identified bug 46479b");
+		}
+		assertEquals(2.5, cell.getNumericCellValue(), 0.0);
+	}
+
+	public void testChangeTypeErrorToBoolean_bug46479() {
+		HSSFCell cell = new HSSFWorkbook().createSheet("Sheet1").createRow(0).createCell(0);
+		cell.setCellErrorValue((byte)HSSFErrorConstants.ERROR_NAME);
+		cell.setCellValue(true); 
+		try {
+			cell.getBooleanCellValue();
+		} catch (IllegalStateException e) {
+			if (e.getMessage().equals("Cannot get a boolean value from a error cell")) {
+
+				throw new AssertionFailedError("Identified bug 46479c");
+			}
+			throw e;
+		}
+		assertEquals(true, cell.getBooleanCellValue());
+	}
+
+	/**
+	 * Test for bug in convertCellValueToBoolean to make sure that formula results get converted 
+	 */
+	public void testChangeTypeFormulaToBoolean_bug46479() {
+		HSSFCell cell = new HSSFWorkbook().createSheet("Sheet1").createRow(0).createCell(0);
+		cell.setCellFormula("1=1");
+		cell.setCellValue(true);
+		cell.setCellType(HSSFCell.CELL_TYPE_BOOLEAN);
+		if (cell.getBooleanCellValue() == false) {
+			throw new AssertionFailedError("Identified bug 46479d");
+		}
+		assertEquals(true, cell.getBooleanCellValue());
+	}
 }
 

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

@@ -17,6 +17,7 @@
 
 package org.apache.poi.hssf.usermodel;
 
+import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
@@ -92,4 +93,32 @@ public final class TestHSSFFormulaEvaluator extends TestCase {
 		HSSFCell cell = sheet.createRow(0).createCell(0);
 		assertNull(fe.evaluate(cell));
 	}
+
+	/**
+	 * Test for bug due to attempt to convert a cached formula error result to a boolean
+	 */
+	public void testUpdateCachedFormulaResultFromErrorToNumber_bug46479() {
+
+		HSSFWorkbook wb = new HSSFWorkbook();
+		HSSFSheet sheet = wb.createSheet("Sheet1");
+		HSSFRow row = sheet.createRow(0);
+		HSSFCell cellA1 = row.createCell(0);
+		HSSFCell cellB1 = row.createCell(1);
+		cellB1.setCellFormula("A1+1");
+		HSSFFormulaEvaluator fe = new HSSFFormulaEvaluator(wb);
+
+		cellA1.setCellErrorValue((byte)HSSFErrorConstants.ERROR_NAME);
+		fe.evaluateFormulaCell(cellB1);
+
+		cellA1.setCellValue(2.5);
+		fe.notifyUpdateCell(cellA1);
+		try {
+			fe.evaluateInCell(cellB1);
+		} catch (IllegalStateException e) {
+			if (e.getMessage().equals("Cannot get a numeric value from a error formula cell")) {
+				throw new AssertionFailedError("Identified bug 46479a");
+			}
+		}
+		assertEquals(3.5, cellB1.getNumericCellValue(), 0.0);
+	}
 }