From: Nick Burch <nick@apache.org>
Date: Wed, 3 Aug 2016 11:53:43 +0000 (+0000)
Subject: Switch XSSF onto BaseFormulaEvaluator, reducing code duplication and fixing XSSFFormu... 
X-Git-Tag: REL_3_15_BETA3~6
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=a57e5b7b932b6f26149c89764bbb39041ea943e5;p=poi.git

Switch XSSF onto BaseFormulaEvaluator, reducing code duplication and fixing XSSFFormulaEvaluator.evaluateAll() to mirror HSSF and use any setup referenced workbooks (stackoverflow #38706562)

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

diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java b/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java
index a20d7cf25a..8d7d781f9a 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java
@@ -33,8 +33,6 @@ import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.ss.usermodel.CellType;
 import org.apache.poi.ss.usermodel.CellValue;
 import org.apache.poi.ss.usermodel.FormulaEvaluator;
-import org.apache.poi.ss.usermodel.Row;
-import org.apache.poi.ss.usermodel.Sheet;
 import org.apache.poi.ss.usermodel.Workbook;
 import org.apache.poi.util.Internal;
 
@@ -248,21 +246,7 @@ public class HSSFFormulaEvaluator extends BaseFormulaEvaluator {
      *  cells, and calling evaluateFormulaCell on each one.
      */
     public static void evaluateAllFormulaCells(Workbook wb) {
-        FormulaEvaluator evaluator = wb.getCreationHelper().createFormulaEvaluator();
-        evaluateAllFormulaCells(wb, evaluator);
-    }
-    private static void evaluateAllFormulaCells(Workbook wb, FormulaEvaluator evaluator) {
-        for(int i=0; i<wb.getNumberOfSheets(); i++) {
-            Sheet sheet = wb.getSheetAt(i);
-
-            for(Row r : sheet) {
-                for (Cell c : r) {
-                    if (c.getCellTypeEnum() == CellType.FORMULA) {
-                        evaluator.evaluateFormulaCellEnum(c);
-                    }
-                }
-            }
-        }
+        BaseFormulaEvaluator.evaluateAllFormulaCells(wb);
     }
 
     /**
diff --git a/src/java/org/apache/poi/ss/formula/BaseFormulaEvaluator.java b/src/java/org/apache/poi/ss/formula/BaseFormulaEvaluator.java
index 787153dd8e..8746ba7fa0 100644
--- a/src/java/org/apache/poi/ss/formula/BaseFormulaEvaluator.java
+++ b/src/java/org/apache/poi/ss/formula/BaseFormulaEvaluator.java
@@ -142,8 +142,10 @@ public abstract class BaseFormulaEvaluator implements FormulaEvaluator, Workbook
                 return;
             case BLANK:
                 // never happens - blanks eventually get translated to zero
+                throw new IllegalArgumentException("This should never happen. Blanks eventually get translated to zero.");
             case FORMULA:
                 // this will never happen, we have already evaluated the formula
+                throw new IllegalArgumentException("This should never happen. Formulas should have already been evaluated.");
             default:
                 throw new IllegalStateException("Unexpected cell value type (" + cellType + ")");
         }
@@ -164,7 +166,7 @@ public abstract class BaseFormulaEvaluator implements FormulaEvaluator, Workbook
         FormulaEvaluator evaluator = wb.getCreationHelper().createFormulaEvaluator();
         evaluateAllFormulaCells(wb, evaluator);
     }
-    private static void evaluateAllFormulaCells(Workbook wb, FormulaEvaluator evaluator) {
+    protected static void evaluateAllFormulaCells(Workbook wb, FormulaEvaluator evaluator) {
         for(int i=0; i<wb.getNumberOfSheets(); i++) {
             Sheet sheet = wb.getSheetAt(i);
 
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFFormulaEvaluator.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFFormulaEvaluator.java
index 5fe8660b1d..c6c030b957 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFFormulaEvaluator.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFFormulaEvaluator.java
@@ -17,12 +17,9 @@
 
 package org.apache.poi.xssf.usermodel;
 
-import java.util.Map;
-
-import org.apache.poi.ss.formula.CollaboratingWorkbooksEnvironment;
+import org.apache.poi.ss.formula.BaseFormulaEvaluator;
 import org.apache.poi.ss.formula.EvaluationCell;
 import org.apache.poi.ss.formula.WorkbookEvaluator;
-import org.apache.poi.ss.formula.WorkbookEvaluatorProvider;
 import org.apache.poi.ss.formula.eval.BoolEval;
 import org.apache.poi.ss.formula.eval.ErrorEval;
 import org.apache.poi.ss.formula.eval.NumberEval;
@@ -31,28 +28,16 @@ import org.apache.poi.ss.formula.eval.ValueEval;
 import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.ss.usermodel.CellType;
 import org.apache.poi.ss.usermodel.CellValue;
-import org.apache.poi.ss.usermodel.FormulaEvaluator;
 import org.apache.poi.util.Internal;
 
 /**
  * Internal POI use only - parent of XSSF and SXSSF formula evaluators
  */
-public abstract class BaseXSSFFormulaEvaluator implements FormulaEvaluator, WorkbookEvaluatorProvider {
-    private WorkbookEvaluator _bookEvaluator;
-
+public abstract class BaseXSSFFormulaEvaluator extends BaseFormulaEvaluator {
     protected BaseXSSFFormulaEvaluator(WorkbookEvaluator bookEvaluator) {
-        _bookEvaluator = bookEvaluator;
+        super(bookEvaluator);
     }
 
-    /**
-     * Should be called whenever there are major changes (e.g. moving sheets) to input cells
-     * in the evaluated workbook.
-     * Failure to call this method after changing cell values will cause incorrect behaviour
-     * of the evaluate~ methods of this class
-     */
-    public void clearAllCachedResultValues() {
-        _bookEvaluator.clearAllCachedResultValues();
-    }
     public void notifySetFormula(Cell cell) {
         _bookEvaluator.notifyUpdateCell(new XSSFEvaluationCell((XSSFCell)cell));
     }
@@ -63,60 +48,6 @@ public abstract class BaseXSSFFormulaEvaluator implements FormulaEvaluator, Work
         _bookEvaluator.notifyUpdateCell(new XSSFEvaluationCell((XSSFCell)cell));
     }
 
-    /**
-     * If cell contains a formula, the formula is evaluated and returned,
-     * else the CellValue simply copies the appropriate cell value from
-     * the cell and also its cell type. This method should be preferred over
-     * evaluateInCell() when the call should not modify the contents of the
-     * original cell.
-     * @param cell
-     */
-    public CellValue evaluate(Cell cell) {
-        if (cell == null) {
-            return null;
-        }
-
-        switch (cell.getCellTypeEnum()) {
-            case BOOLEAN:
-                return CellValue.valueOf(cell.getBooleanCellValue());
-            case ERROR:
-                return CellValue.getError(cell.getErrorCellValue());
-            case FORMULA:
-                return evaluateFormulaCellValue(cell);
-            case NUMERIC:
-                return new CellValue(cell.getNumericCellValue());
-            case STRING:
-                return new CellValue(cell.getRichStringCellValue().getString());
-            case BLANK:
-                return null;
-            default:
-                throw new IllegalStateException("Bad cell type (" + cell.getCellTypeEnum() + ")");
-        }
-    }
-
-
-    /**
-     * If cell contains formula, it evaluates the formula,
-     *  and saves the result of the formula. The cell
-     *  remains as a formula cell.
-     * Else if cell does not contain formula, this method leaves
-     *  the cell unchanged.
-     * Note that the type of the formula result is returned,
-     *  so you know what kind of value is also stored with
-     *  the formula.
-     * <pre>
-     * int evaluatedCellType = evaluator.evaluateFormulaCell(cell);
-     * </pre>
-     * Be aware that your cell will hold both the formula,
-     *  and the result. If you want the cell replaced with
-     *  the result of the formula, use {@link #evaluate(org.apache.poi.ss.usermodel.Cell)} }
-     * @param cell The cell to evaluate
-     * @return The type of the formula result (the cell's type remains as CellType.FORMULA however)
-     */
-    public int evaluateFormulaCell(Cell cell) {
-        return evaluateFormulaCellEnum(cell).getCode();
-    }
-    
     /**
      * If cell contains formula, it evaluates the formula,
      *  and saves the result of the formula. The cell
@@ -164,27 +95,6 @@ public abstract class BaseXSSFFormulaEvaluator implements FormulaEvaluator, Work
             setCellValue(cell, cv);
         }
     }
-    private static void setCellType(Cell cell, CellValue cv) {
-        CellType cellType = cv.getCellType();
-        switch (cellType) {
-            case BOOLEAN:
-            case ERROR:
-            case NUMERIC:
-            case STRING:
-                cell.setCellType(cellType);
-                return;
-            case BLANK:
-                // never happens - blanks eventually get translated to zero
-                throw new IllegalArgumentException("This should never happen. Blanks eventually get translated to zero.");
-            case FORMULA:
-                // this will never happen, we have already evaluated the formula
-                throw new IllegalArgumentException("This should never happen. Formulas should have already been evaluated.");
-            default:
-                throw new IllegalStateException("Unexpected cell value type (" + cellType + ")");
-            
-        }
-        
-    }
 
     private static void setCellValue(Cell cell, CellValue cv) {
         CellType cellType = cv.getCellType();
@@ -218,7 +128,7 @@ public abstract class BaseXSSFFormulaEvaluator implements FormulaEvaluator, Work
     /**
      * Returns a CellValue wrapper around the supplied ValueEval instance.
      */
-    private CellValue evaluateFormulaCellValue(Cell cell) {
+    protected CellValue evaluateFormulaCellValue(Cell cell) {
         EvaluationCell evalCell = toEvaluationCell(cell);
         ValueEval eval = _bookEvaluator.evaluate(evalCell);
         if (eval instanceof NumberEval) {
@@ -238,22 +148,4 @@ public abstract class BaseXSSFFormulaEvaluator implements FormulaEvaluator, Work
         }
         throw new RuntimeException("Unexpected eval class (" + eval.getClass().getName() + ")");
     }
-
-    public void setupReferencedWorkbooks(Map<String, FormulaEvaluator> evaluators) {
-        CollaboratingWorkbooksEnvironment.setupFormulaEvaluator(evaluators);
-    }
-
-    public WorkbookEvaluator _getWorkbookEvaluator() {
-        return _bookEvaluator;
-    }
-
-    /** {@inheritDoc} */
-    public void setIgnoreMissingWorkbooks(boolean ignore){
-        _bookEvaluator.setIgnoreMissingWorkbooks(ignore);
-    }
-
-    /** {@inheritDoc} */
-    public void setDebugEvaluationOutputForNextEval(boolean value){
-        _bookEvaluator.setDebugEvaluationOutputForNextEval(value);
-    }
 }
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFormulaEvaluator.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFormulaEvaluator.java
index ec9ecd4621..fc456b7e58 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFormulaEvaluator.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFormulaEvaluator.java
@@ -17,7 +17,7 @@
 
 package org.apache.poi.xssf.usermodel;
 
-import org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator;
+import org.apache.poi.ss.formula.BaseFormulaEvaluator;
 import org.apache.poi.ss.formula.EvaluationCell;
 import org.apache.poi.ss.formula.IStabilityClassifier;
 import org.apache.poi.ss.formula.WorkbookEvaluator;
@@ -88,7 +88,7 @@ public final class XSSFFormulaEvaluator extends BaseXSSFFormulaEvaluator {
      *  cells, and calling evaluateFormulaCell on each one.
      */
     public static void evaluateAllFormulaCells(XSSFWorkbook wb) {
-        HSSFFormulaEvaluator.evaluateAllFormulaCells(wb);
+        BaseFormulaEvaluator.evaluateAllFormulaCells(wb);
     }
     /**
      * Loops over all cells in all sheets of the supplied
@@ -102,7 +102,7 @@ public final class XSSFFormulaEvaluator extends BaseXSSFFormulaEvaluator {
      *  cells, and calling evaluateFormulaCell on each one.
      */
     public void evaluateAll() {
-        HSSFFormulaEvaluator.evaluateAllFormulaCells(_book);
+        evaluateAllFormulaCells(_book, this);
     }
 
     /**
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java
index 9885fd8863..6dcaef9603 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java
@@ -173,8 +173,19 @@ public final class TestXSSFFormulaEvaluation extends BaseTestFormulaEvaluator {
                 evaluator.evaluate(c);
             }
         }
+        // And evaluate the other way too
+        evaluator.evaluateAll();
         
-        // Evaluate and check results
+        // Static evaluator won't work, as no references passed in
+        try {
+            XSSFFormulaEvaluator.evaluateAllFormulaCells(wb);
+            fail("Static method lacks references, shouldn't work");
+        } catch(Exception e) {
+            // expected here
+        }
+        
+        
+        // Evaluate specific cells and check results
         assertEquals("\"Hello!\"",  evaluator.evaluate(cXSLX_cell).formatAsString());
         assertEquals("\"Test A1\"", evaluator.evaluate(cXSLX_sNR).formatAsString());
         assertEquals("142.0",   evaluator.evaluate(cXSLX_gNR).formatAsString());