fixed XSSFFormulaEvaluator to support blank cells, added BaseTestFormulaEvaluator - a common superclass for testing implementations of FormulaEvaluator

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

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 
     <changes>
         <release version="3.7-beta3" date="2010-??-??">
+           <action dev="poi-developers" type="fix">49873 - fixed  XSSFFormulaEvaluator to support blank cells</action>
            <action dev="poi-developers" type="fix">49850 - added a getter for _iStartAt in ListFormatOverrideLevel</action>
            <action dev="poi-developers" type="fix">49761 - change cell type to error when setting Double.NaN or Infinities</action>
            <action dev="poi-developers" type="fix">49833 - ensure that CTNumPr is included in poi-ooxml-schemas.jar</action>

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

@@ -132,6 +132,9 @@ public class HSSFFormulaEvaluator implements FormulaEvaluator  {
 	public void notifyUpdateCell(HSSFCell cell) {
 		_bookEvaluator.notifyUpdateCell(new HSSFEvaluationCell(cell));
 	}
+    public void notifyUpdateCell(Cell cell) {
+        _bookEvaluator.notifyUpdateCell(new HSSFEvaluationCell((HSSFCell)cell));
+    }
 	/**
 	 * Should be called to tell the cell value cache that the specified cell has just been
 	 * deleted.

src/java/org/apache/poi/ss/usermodel/FormulaEvaluator.java

@@ -47,6 +47,15 @@ public interface FormulaEvaluator {
 	 * formula cell, or the formula text has changed 
 	 */
     void notifyDeleteCell(Cell cell);
+
+    /**
+     * Should be called to tell the cell value cache that the specified (value or formula) cell
+     * has changed.
+     * Failure to call this method after changing cell values will cause incorrect behaviour
+     * of the evaluate~ methods of this class
+     */
+    void notifyUpdateCell(Cell cell);
+
     /**
      * If cell contains a formula, the formula is evaluated and returned,
      * else the CellValue simply copies the appropriate cell value from

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFormulaEvaluator.java

@@ -90,6 +90,9 @@ public class XSSFFormulaEvaluator implements FormulaEvaluator {
 	public void notifyDeleteCell(Cell cell) {
 		_bookEvaluator.notifyDeleteCell(new XSSFEvaluationCell((XSSFCell)cell));
 	}
+    public void notifyUpdateCell(Cell cell) {
+        _bookEvaluator.notifyUpdateCell(new XSSFEvaluationCell((XSSFCell)cell));
+    }
 
 	/**
 	 * If cell contains a formula, the formula is evaluated and returned,
@@ -115,6 +118,8 @@ public class XSSFFormulaEvaluator implements FormulaEvaluator {
 				return new CellValue(cell.getNumericCellValue());
 			case XSSFCell.CELL_TYPE_STRING:
 				return new CellValue(cell.getRichStringCellValue().getString());
+            case XSSFCell.CELL_TYPE_BLANK:
+                return null;
 		}
 		throw new IllegalStateException("Bad cell type (" + cell.getCellType() + ")");
 	}

src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java

@@ -21,139 +21,15 @@ import junit.framework.TestCase;
 
 import org.apache.poi.ss.usermodel.*;
 import org.apache.poi.xssf.XSSFTestDataSamples;
+import org.apache.poi.xssf.XSSFITestDataProvider;
 
-public final class TestXSSFFormulaEvaluation extends TestCase {
+public final class TestXSSFFormulaEvaluation extends BaseTestFormulaEvaluator {
-	@Override
-	protected void setUp() {
-        // Use system out logger
-        System.setProperty(
-                "org.apache.poi.util.POILogger",
-                "org.apache.poi.util.SystemOutLogger"
-        );
-    }
 
-    public void testSimpleArithmatic() {
+    public TestXSSFFormulaEvaluation() {
-        XSSFWorkbook wb = new XSSFWorkbook();
+        super(XSSFITestDataProvider.instance);
-        Sheet s = wb.createSheet();
-        Row r = s.createRow(0);
-
-        Cell c1 = r.createCell(0);
-        c1.setCellFormula("1+5");
-        assertEquals(0.0, c1.getNumericCellValue(), 0.0);
-
-        Cell c2 = r.createCell(1);
-        c2.setCellFormula("10/2");
-        assertEquals(0.0, c2.getNumericCellValue(), 0.0);
-
-        FormulaEvaluator fe = new XSSFFormulaEvaluator(wb);
-
-        fe.evaluateFormulaCell(c1);
-        fe.evaluateFormulaCell(c2);
-
-        assertEquals(6.0, c1.getNumericCellValue(), 0.0001);
-        assertEquals(5.0, c2.getNumericCellValue(), 0.0001);
-    }
-
-    public void testSumCount() {
-        XSSFWorkbook wb = new XSSFWorkbook();
-        Sheet s = wb.createSheet();
-        Row r = s.createRow(0);
-        r.createCell(0).setCellValue(2.5);
-        r.createCell(1).setCellValue(1.1);
-        r.createCell(2).setCellValue(3.2);
-        r.createCell(4).setCellValue(10.7);
-
-        r = s.createRow(1);
-
-        Cell c1 = r.createCell(0);
-        c1.setCellFormula("SUM(A1:B1)");
-        assertEquals(0.0, c1.getNumericCellValue(), 0.0);
-
-        Cell c2 = r.createCell(1);
-        c2.setCellFormula("SUM(A1:E1)");
-        assertEquals(0.0, c2.getNumericCellValue(), 0.0);
-
-        Cell c3 = r.createCell(2);
-        c3.setCellFormula("COUNT(A1:A1)");
-        assertEquals(0.0, c3.getNumericCellValue(), 0.0);
-
-        Cell c4 = r.createCell(3);
-        c4.setCellFormula("COUNTA(A1:E1)");
-        assertEquals(0.0, c4.getNumericCellValue(), 0.0);
-
-
-        // Evaluate and test
-        FormulaEvaluator fe = new XSSFFormulaEvaluator(wb);
-
-        fe.evaluateFormulaCell(c1);
-        fe.evaluateFormulaCell(c2);
-        fe.evaluateFormulaCell(c3);
-        fe.evaluateFormulaCell(c4);
-
-        assertEquals(3.6, c1.getNumericCellValue(), 0.0001);
-        assertEquals(17.5, c2.getNumericCellValue(), 0.0001);
-        assertEquals(1, c3.getNumericCellValue(), 0.0001);
-        assertEquals(4, c4.getNumericCellValue(), 0.0001);
     }
 
     public void testSharedFormulas(){
-        XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("shared_formulas.xlsx");
+        baseTestSharedFormulas("shared_formulas.xlsx");
-
-        XSSFSheet sheet = wb.getSheetAt(0);
-
-        FormulaEvaluator evaluator = wb.getCreationHelper().createFormulaEvaluator();
-        XSSFCell cell;
-
-        cell = sheet.getRow(1).getCell(0);
-        assertEquals("B2", cell.getCellFormula());
-        assertEquals("ProductionOrderConfirmation", evaluator.evaluate(cell).getStringValue());
-
-        cell = sheet.getRow(2).getCell(0);
-        assertEquals("B3", cell.getCellFormula());
-        assertEquals("RequiredAcceptanceDate", evaluator.evaluate(cell).getStringValue());
-
-        cell = sheet.getRow(3).getCell(0);
-        assertEquals("B4", cell.getCellFormula());
-        assertEquals("Header", evaluator.evaluate(cell).getStringValue());
-
-        cell = sheet.getRow(4).getCell(0);
-        assertEquals("B5", cell.getCellFormula());
-        assertEquals("UniqueDocumentNumberID", evaluator.evaluate(cell).getStringValue());
-    }
-
-    /**
-     * Test creation / evaluation of formulas with sheet-level names
-     */
-    public void testSheetLevelFormulas(){
-        XSSFWorkbook wb = new XSSFWorkbook();
-
-        XSSFRow row;
-        XSSFSheet sh1 = wb.createSheet("Sheet1");
-        XSSFName nm1 = wb.createName();
-        nm1.setNameName("sales_1");
-        nm1.setSheetIndex(0);
-        nm1.setRefersToFormula("Sheet1!$A$1");
-        row = sh1.createRow(0);
-        row.createCell(0).setCellValue(3);
-        row.createCell(1).setCellFormula("sales_1");
-        row.createCell(2).setCellFormula("sales_1*2");
-
-        XSSFSheet sh2 = wb.createSheet("Sheet2");
-        XSSFName nm2 = wb.createName();
-        nm2.setNameName("sales_1");
-        nm2.setSheetIndex(1);
-        nm2.setRefersToFormula("Sheet2!$A$1");
-
-        row = sh2.createRow(0);
-        row.createCell(0).setCellValue(5);
-        row.createCell(1).setCellFormula("sales_1");
-        row.createCell(2).setCellFormula("sales_1*3");
-
-        XSSFFormulaEvaluator evaluator = new XSSFFormulaEvaluator(wb);
-        assertEquals(3.0, evaluator.evaluate(sh1.getRow(0).getCell(1)).getNumberValue(), 0.0);
-        assertEquals(6.0, evaluator.evaluate(sh1.getRow(0).getCell(2)).getNumberValue(), 0.0);
-
-        assertEquals(5.0, evaluator.evaluate(sh2.getRow(0).getCell(1)).getNumberValue(), 0.0);
-        assertEquals(15.0, evaluator.evaluate(sh2.getRow(0).getCell(2)).getNumberValue(), 0.0);
     }
 }

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

@@ -21,6 +21,7 @@ import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
+import org.apache.poi.hssf.HSSFITestDataProvider;
 import org.apache.poi.hssf.record.NameRecord;
 import org.apache.poi.hssf.record.formula.eval.NumberEval;
 import org.apache.poi.hssf.record.formula.eval.ValueEval;
@@ -30,11 +31,17 @@ import org.apache.poi.ss.formula.WorkbookEvaluator;
 import org.apache.poi.ss.formula.WorkbookEvaluatorTestHelper;
 import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.ss.usermodel.CellValue;
+import org.apache.poi.ss.usermodel.BaseTestFormulaEvaluator;
+
 /**
  *
  * @author Josh Micich
  */
-public final class TestHSSFFormulaEvaluator extends TestCase {
+public final class TestHSSFFormulaEvaluator extends BaseTestFormulaEvaluator {
+
+    public TestHSSFFormulaEvaluator() {
+        super(HSSFITestDataProvider.instance);
+    }
 
 	/**
 	 * Test that the HSSFFormulaEvaluator can evaluate simple named ranges
@@ -50,58 +57,6 @@ public final class TestHSSFFormulaEvaluator extends TestCase {
 		assertEquals(3.72, cv.getNumberValue(), 0.0);
 	}
 
-	public void testFullColumnRefs() {
-		HSSFWorkbook wb = new HSSFWorkbook();
-		HSSFSheet sheet = wb.createSheet("Sheet1");
-		HSSFRow row = sheet.createRow(0);
-		HSSFCell cell0 = row.createCell(0);
-		cell0.setCellFormula("sum(D:D)");
-		HSSFCell cell1 = row.createCell(1);
-		cell1.setCellFormula("sum(D:E)");
-
-		// some values in column D
-		setValue(sheet, 1, 3, 5.0);
-		setValue(sheet, 2, 3, 6.0);
-		setValue(sheet, 5, 3, 7.0);
-		setValue(sheet, 50, 3, 8.0);
-
-		// some values in column E
-		setValue(sheet, 1, 4, 9.0);
-		setValue(sheet, 2, 4, 10.0);
-		setValue(sheet, 30000, 4, 11.0);
-
-		// some other values
-		setValue(sheet, 1, 2, 100.0);
-		setValue(sheet, 2, 5, 100.0);
-		setValue(sheet, 3, 6, 100.0);
-
-
-		HSSFFormulaEvaluator fe = new HSSFFormulaEvaluator(wb);
-		assertEquals(26.0, fe.evaluate(cell0).getNumberValue(), 0.0);
-		assertEquals(56.0, fe.evaluate(cell1).getNumberValue(), 0.0);
-	}
-
-	private static void setValue(HSSFSheet sheet, int rowIndex, int colIndex, double value) {
-		HSSFRow row = sheet.getRow(rowIndex);
-		if (row == null) {
-			row = sheet.createRow(rowIndex);
-		}
-		row.createCell(colIndex).setCellValue(value);
-	}
-
-	/**
-	 * {@link HSSFFormulaEvaluator#evaluate(org.apache.poi.ss.usermodel.Cell)} should behave the same whether the cell
-	 * is <code>null</code> or blank.
-	 */
-	public void testEvaluateBlank() {
-		HSSFWorkbook wb = new HSSFWorkbook();
-		HSSFFormulaEvaluator fe = new HSSFFormulaEvaluator(wb);
-		assertNull(fe.evaluate(null));
-		HSSFSheet sheet = wb.createSheet("Sheet1");
-		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
 	 */
@@ -276,4 +231,9 @@ public final class TestHSSFFormulaEvaluator extends TestCase {
       assertEquals(Cell.CELL_TYPE_NUMERIC, cell.getCachedFormulaResultType());
       assertEquals(36.90, cell.getNumericCellValue(), 0.0001);
    }
+
+    public void testSharedFormulas(){
+        baseTestSharedFormulas("shared_formulas.xls");
+    }
+
 }

src/testcases/org/apache/poi/ss/usermodel/BaseTestFormulaEvaluator.java

@@ -0,0 +1,246 @@
+/* ====================================================================
+   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.ss.usermodel;
+
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
+import org.apache.poi.ss.ITestDataProvider;
+
+/**
+ * Common superclass for testing implementatiosn of{@link FormulaEvaluator}
+ *
+ * @author Yegor Kozlov
+ */
+public abstract class BaseTestFormulaEvaluator extends TestCase {
+
+	protected final ITestDataProvider _testDataProvider;
+
+	/**
+	 * @param testDataProvider an object that provides test data in  /  specific way
+	 */
+	protected BaseTestFormulaEvaluator(ITestDataProvider testDataProvider) {
+		_testDataProvider = testDataProvider;
+	}
+
+    public void testSimpleArithmetic() {
+        Workbook wb = _testDataProvider.createWorkbook();
+        Sheet s = wb.createSheet();
+        Row r = s.createRow(0);
+
+        Cell c1 = r.createCell(0);
+        c1.setCellFormula("1+5");
+        assertEquals(0.0, c1.getNumericCellValue(), 0.0);
+
+        Cell c2 = r.createCell(1);
+        c2.setCellFormula("10/2");
+        assertEquals(0.0, c2.getNumericCellValue(), 0.0);
+
+        FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator();
+
+        fe.evaluateFormulaCell(c1);
+        fe.evaluateFormulaCell(c2);
+
+        assertEquals(6.0, c1.getNumericCellValue(), 0.0001);
+        assertEquals(5.0, c2.getNumericCellValue(), 0.0001);
+    }
+
+    public void testSumCount() {
+        Workbook wb = _testDataProvider.createWorkbook();
+        Sheet s = wb.createSheet();
+        Row r = s.createRow(0);
+        r.createCell(0).setCellValue(2.5);
+        r.createCell(1).setCellValue(1.1);
+        r.createCell(2).setCellValue(3.2);
+        r.createCell(4).setCellValue(10.7);
+
+        r = s.createRow(1);
+
+        Cell c1 = r.createCell(0);
+        c1.setCellFormula("SUM(A1:B1)");
+        assertEquals(0.0, c1.getNumericCellValue(), 0.0);
+
+        Cell c2 = r.createCell(1);
+        c2.setCellFormula("SUM(A1:E1)");
+        assertEquals(0.0, c2.getNumericCellValue(), 0.0);
+
+        Cell c3 = r.createCell(2);
+        c3.setCellFormula("COUNT(A1:A1)");
+        assertEquals(0.0, c3.getNumericCellValue(), 0.0);
+
+        Cell c4 = r.createCell(3);
+        c4.setCellFormula("COUNTA(A1:E1)");
+        assertEquals(0.0, c4.getNumericCellValue(), 0.0);
+
+
+        // Evaluate and test
+        FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator();
+
+        fe.evaluateFormulaCell(c1);
+        fe.evaluateFormulaCell(c2);
+        fe.evaluateFormulaCell(c3);
+        fe.evaluateFormulaCell(c4);
+
+        assertEquals(3.6, c1.getNumericCellValue(), 0.0001);
+        assertEquals(17.5, c2.getNumericCellValue(), 0.0001);
+        assertEquals(1, c3.getNumericCellValue(), 0.0001);
+        assertEquals(4, c4.getNumericCellValue(), 0.0001);
+    }
+
+    public void baseTestSharedFormulas(String sampleFile){
+        Workbook wb = _testDataProvider.openSampleWorkbook(sampleFile);
+
+        Sheet sheet = wb.getSheetAt(0);
+
+        FormulaEvaluator evaluator = wb.getCreationHelper().createFormulaEvaluator();
+        Cell cell;
+
+        cell = sheet.getRow(1).getCell(0);
+        assertEquals("B2", cell.getCellFormula());
+        assertEquals("ProductionOrderConfirmation", evaluator.evaluate(cell).getStringValue());
+
+        cell = sheet.getRow(2).getCell(0);
+        assertEquals("B3", cell.getCellFormula());
+        assertEquals("RequiredAcceptanceDate", evaluator.evaluate(cell).getStringValue());
+
+        cell = sheet.getRow(3).getCell(0);
+        assertEquals("B4", cell.getCellFormula());
+        assertEquals("Header", evaluator.evaluate(cell).getStringValue());
+
+        cell = sheet.getRow(4).getCell(0);
+        assertEquals("B5", cell.getCellFormula());
+        assertEquals("UniqueDocumentNumberID", evaluator.evaluate(cell).getStringValue());
+    }
+
+    /**
+     * Test creation / evaluation of formulas with sheet-level names
+     */
+    public void testSheetLevelFormulas(){
+        Workbook wb = _testDataProvider.createWorkbook();
+
+        Row row;
+        Sheet sh1 = wb.createSheet("Sheet1");
+        Name nm1 = wb.createName();
+        nm1.setNameName("sales_1");
+        nm1.setSheetIndex(0);
+        nm1.setRefersToFormula("Sheet1!$A$1");
+        row = sh1.createRow(0);
+        row.createCell(0).setCellValue(3);
+        row.createCell(1).setCellFormula("sales_1");
+        row.createCell(2).setCellFormula("sales_1*2");
+
+        Sheet sh2 = wb.createSheet("Sheet2");
+        Name nm2 = wb.createName();
+        nm2.setNameName("sales_1");
+        nm2.setSheetIndex(1);
+        nm2.setRefersToFormula("Sheet2!$A$1");
+
+        row = sh2.createRow(0);
+        row.createCell(0).setCellValue(5);
+        row.createCell(1).setCellFormula("sales_1");
+        row.createCell(2).setCellFormula("sales_1*3");
+
+        FormulaEvaluator evaluator = wb.getCreationHelper().createFormulaEvaluator();
+        assertEquals(3.0, evaluator.evaluate(sh1.getRow(0).getCell(1)).getNumberValue(), 0.0);
+        assertEquals(6.0, evaluator.evaluate(sh1.getRow(0).getCell(2)).getNumberValue(), 0.0);
+
+        assertEquals(5.0, evaluator.evaluate(sh2.getRow(0).getCell(1)).getNumberValue(), 0.0);
+        assertEquals(15.0, evaluator.evaluate(sh2.getRow(0).getCell(2)).getNumberValue(), 0.0);
+    }
+
+    public void testFullColumnRefs() {
+        Workbook wb = _testDataProvider.createWorkbook();
+        Sheet sheet = wb.createSheet("Sheet1");
+        Row row = sheet.createRow(0);
+        Cell cell0 = row.createCell(0);
+        cell0.setCellFormula("sum(D:D)");
+        Cell cell1 = row.createCell(1);
+        cell1.setCellFormula("sum(D:E)");
+
+        // some values in column D
+        setValue(sheet, 1, 3, 5.0);
+        setValue(sheet, 2, 3, 6.0);
+        setValue(sheet, 5, 3, 7.0);
+        setValue(sheet, 50, 3, 8.0);
+
+        // some values in column E
+        setValue(sheet, 1, 4, 9.0);
+        setValue(sheet, 2, 4, 10.0);
+        setValue(sheet, 30000, 4, 11.0);
+
+        // some other values
+        setValue(sheet, 1, 2, 100.0);
+        setValue(sheet, 2, 5, 100.0);
+        setValue(sheet, 3, 6, 100.0);
+
+
+        FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator();
+        assertEquals(26.0, fe.evaluate(cell0).getNumberValue(), 0.0);
+        assertEquals(56.0, fe.evaluate(cell1).getNumberValue(), 0.0);
+    }
+
+    private static void setValue(Sheet sheet, int rowIndex, int colIndex, double value) {
+        Row row = sheet.getRow(rowIndex);
+        if (row == null) {
+            row = sheet.createRow(rowIndex);
+        }
+        row.createCell(colIndex).setCellValue(value);
+    }
+
+    /**
+     * {@link FormulaEvaluator#evaluate(org.apache.poi.ss.usermodel.Cell)} should behave the same whether the cell
+     * is <code>null</code> or blank.
+     */
+    public void testEvaluateBlank() {
+        Workbook wb = _testDataProvider.createWorkbook();
+        FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator();
+        assertNull(fe.evaluate(null));
+        Sheet sheet = wb.createSheet("Sheet1");
+        Cell 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() {
+
+        Workbook wb = _testDataProvider.createWorkbook();
+        Sheet sheet = wb.createSheet("Sheet1");
+        Row row = sheet.createRow(0);
+        Cell cellA1 = row.createCell(0);
+        Cell cellB1 = row.createCell(1);
+        cellB1.setCellFormula("A1+1");
+        FormulaEvaluator fe = wb.getCreationHelper().createFormulaEvaluator();
+
+        cellA1.setCellErrorValue((byte)ErrorConstants.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);
+    }
+
+}