You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by gw...@apache.org on 2018/10/19 01:11:47 UTC

svn commit: r1844295 - in /poi/trunk: src/java/org/apache/poi/ss/formula/ src/java/org/apache/poi/ss/usermodel/ src/ooxml/java/org/apache/poi/xssf/usermodel/ src/ooxml/testcases/org/apache/poi/xssf/usermodel/ test-data/spreadsheet/

Author: gwoolsey
Date: Fri Oct 19 01:11:47 2018
New Revision: 1844295

URL: http://svn.apache.org/viewvc?rev=1844295&view=rev
Log:
#62834 FormulaEvaluator.evaluateInCell() throws Exception

added cell type = formula check when looping through the shared formula range, to ignore any non-formula cells.

Also refactored a bit to enable passing in the evaluation context, as getCellFormula() uses it behind the scenes when evaluating a shared formula cell (has to shift the formula references based on the master cell).  Review of these changes is welcome, as always.

Checked all other code referencing the "SHARED" enum, and didn't see anything else that dealt with formula cell values and thus would need to notice non-formula cells.

Added unit test based on the failing file from Bugzilla.  Test failed until the fixed code was in place.

Added:
    poi/trunk/test-data/spreadsheet/62834.xlsx   (with props)
Modified:
    poi/trunk/src/java/org/apache/poi/ss/formula/BaseFormulaEvaluator.java
    poi/trunk/src/java/org/apache/poi/ss/usermodel/Sheet.java
    poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFFormulaEvaluator.java
    poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java
    poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
    poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java

Modified: poi/trunk/src/java/org/apache/poi/ss/formula/BaseFormulaEvaluator.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/BaseFormulaEvaluator.java?rev=1844295&r1=1844294&r2=1844295&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/BaseFormulaEvaluator.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/BaseFormulaEvaluator.java Fri Oct 19 01:11:47 2018
@@ -65,6 +65,14 @@ public abstract class BaseFormulaEvaluat
     }
 
     /**
+     * internal use
+     * @return evaluation workbook
+     */
+    protected EvaluationWorkbook getEvaluationWorkbook() {
+        return _bookEvaluator.getWorkbook();
+    }
+    
+    /**
      * Should be called whenever there are major changes (e.g. moving sheets) to input cells
      * in the evaluated workbook.  If performance is not critical, a single call to this method
      * may be used instead of many specific calls to the notify~ methods.
@@ -208,14 +216,19 @@ public abstract class BaseFormulaEvaluat
         return evaluateFormulaCell(cell);
     }
 
-    protected static void setCellType(Cell cell, CellValue cv) {
+    /**
+     * set the cell type
+     * @param cell
+     * @param cv
+     */
+    protected void setCellType(Cell cell, CellValue cv) {
         CellType cellType = cv.getCellType();
         switch (cellType) {
             case BOOLEAN:
             case ERROR:
             case NUMERIC:
             case STRING:
-                cell.setCellType(cellType);
+                setCellType(cell, cellType);
                 return;
             case BLANK:
                 // never happens - blanks eventually get translated to zero
@@ -227,6 +240,15 @@ public abstract class BaseFormulaEvaluat
                 throw new IllegalStateException("Unexpected cell value type (" + cellType + ")");
         }
     }
+
+    /**
+     * Override if a different variation is needed, e.g. passing the evaluator to the cell method
+     * @param cell
+     * @param cellType
+     */
+    protected void setCellType(Cell cell, CellType cellType) {
+        cell.setCellType(cellType);
+    }
     
     protected abstract RichTextString createRichTextString(String str);
     

Modified: poi/trunk/src/java/org/apache/poi/ss/usermodel/Sheet.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/usermodel/Sheet.java?rev=1844295&r1=1844294&r2=1844295&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/usermodel/Sheet.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/usermodel/Sheet.java Fri Oct 19 01:11:47 2018
@@ -1006,7 +1006,10 @@ public interface Sheet extends Iterable<
 
     /**
      * Sets array formula to specified region for result.
-     *
+     * <p>
+     * Note if there are shared formulas this will invalidate any 
+     * {@link FormulaEvaluator} instances based on this workbook
+     *</p>
      * @param formula text representation of the formula
      * @param range Region of array formula for result.
      * @return the {@link CellRange} of cells affected by this change

Modified: poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFFormulaEvaluator.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFFormulaEvaluator.java?rev=1844295&r1=1844294&r2=1844295&view=diff
==============================================================================
--- poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFFormulaEvaluator.java (original)
+++ poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFFormulaEvaluator.java Fri Oct 19 01:11:47 2018
@@ -19,6 +19,7 @@ package org.apache.poi.xssf.usermodel;
 
 import org.apache.poi.ss.formula.BaseFormulaEvaluator;
 import org.apache.poi.ss.formula.EvaluationCell;
+import org.apache.poi.ss.formula.EvaluationWorkbook;
 import org.apache.poi.ss.formula.WorkbookEvaluator;
 import org.apache.poi.ss.formula.eval.BoolEval;
 import org.apache.poi.ss.formula.eval.ErrorEval;
@@ -26,6 +27,7 @@ import org.apache.poi.ss.formula.eval.Nu
 import org.apache.poi.ss.formula.eval.StringEval;
 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.RichTextString;
 
@@ -69,4 +71,11 @@ public abstract class BaseXSSFFormulaEva
         }
         throw new RuntimeException("Unexpected eval class (" + eval.getClass().getName() + ")");
     }
+    
+    protected void setCellType(Cell cell, CellType cellType) {
+        EvaluationWorkbook evaluationWorkbook = getEvaluationWorkbook();
+        BaseXSSFEvaluationWorkbook xewb = BaseXSSFEvaluationWorkbook.class.isAssignableFrom(evaluationWorkbook.getClass()) ? (BaseXSSFEvaluationWorkbook) evaluationWorkbook : null;
+        
+        ((XSSFCell) cell).setCellType(cellType, xewb);
+    }
 }

Modified: poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java?rev=1844295&r1=1844294&r2=1844295&view=diff
==============================================================================
--- poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java (original)
+++ poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java Fri Oct 19 01:11:47 2018
@@ -37,6 +37,7 @@ import org.apache.poi.ss.usermodel.Comme
 import org.apache.poi.ss.usermodel.DataFormatter;
 import org.apache.poi.ss.usermodel.DateUtil;
 import org.apache.poi.ss.usermodel.FormulaError;
+import org.apache.poi.ss.usermodel.FormulaEvaluator;
 import org.apache.poi.ss.usermodel.Hyperlink;
 import org.apache.poi.ss.usermodel.RichTextString;
 import org.apache.poi.ss.usermodel.Row.MissingCellPolicy;
@@ -477,7 +478,7 @@ public final class XSSFCell implements C
      * @return a formula for the cell
      * @throws IllegalStateException if the cell type returned by {@link #getCellType()} is not {@link CellType#FORMULA}
      */
-    protected String getCellFormula(XSSFEvaluationWorkbook fpb) {
+    protected String getCellFormula(BaseXSSFEvaluationWorkbook fpb) {
         CellType cellType = getCellType();
         if(cellType != CellType.FORMULA) {
             throw typeMismatch(CellType.FORMULA, cellType, false);
@@ -506,7 +507,7 @@ public final class XSSFCell implements C
      * @param si Shared Group Index
      * @return non shared formula created for the given shared formula and this cell
      */
-    private String convertSharedFormula(int si, XSSFEvaluationWorkbook fpb){
+    private String convertSharedFormula(int si, BaseXSSFEvaluationWorkbook fpb){
         XSSFSheet sheet = getSheet();
 
         CTCellFormula f = sheet.getSharedFormula(si);
@@ -536,6 +537,10 @@ public final class XSSFCell implements C
      * Note, this method only sets the formula string and does not calculate the formula value.
      * To set the precalculated value use {@link #setCellValue(double)} or {@link #setCellValue(String)}
      * </p>
+     * <p>
+     * Note, if there are any shared formulas, his will invalidate any 
+     * {@link FormulaEvaluator} instances based on this workbook.
+     * </p>
      *
      * @param formula the formula to set, e.g. <code>"SUM(C4:E4)"</code>.
      *  If the argument is <code>null</code> then the current formula is removed.
@@ -563,7 +568,7 @@ public final class XSSFCell implements C
         if (formula == null) {
             wb.onDeleteFormula(this);
             if (_cell.isSetF()) {
-                _row.getSheet().onDeleteFormula(this);
+                _row.getSheet().onDeleteFormula(this, null);
                 _cell.unsetF();
             }
             return;
@@ -962,6 +967,16 @@ public final class XSSFCell implements C
      */
     @Override
     public void setCellType(CellType cellType) {
+        setCellType(cellType, null);
+    }
+    
+    /**
+     * Needed by bug #62834, which points out getCellFormula() expects an evaluation context or creates a new one,
+     * so if there is one in use, it needs to be carried on through.
+     * @param cellType
+     * @param evalWb BaseXSSFEvaluationWorkbook already in use, or null if a new implicit one should be used
+     */
+    protected void setCellType(CellType cellType, BaseXSSFEvaluationWorkbook evalWb) {
         CellType prevType = getCellType();
 
         if(isPartOfArrayFormulaGroup()){
@@ -969,7 +984,7 @@ public final class XSSFCell implements C
         }
         if(prevType == CellType.FORMULA && cellType != CellType.FORMULA) {
             if (_cell.isSetF()) {
-                _row.getSheet().onDeleteFormula(this);
+                _row.getSheet().onDeleteFormula(this, evalWb);
             }
             getSheet().getWorkbook().onDeleteFormula(this);
         }

Modified: poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java?rev=1844295&r1=1844294&r2=1844295&view=diff
==============================================================================
--- poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java (original)
+++ poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java Fri Oct 19 01:11:47 2018
@@ -4621,8 +4621,10 @@ public class XSSFSheet extends POIXMLDoc
 
     /**
      *  when a cell with a 'master' shared formula is removed,  the next cell in the range becomes the master
+     * @param cell 
+     * @param evalWb BaseXSSFEvaluationWorkbook in use, if one exists
      */
-    protected void onDeleteFormula(XSSFCell cell){
+    protected void onDeleteFormula(XSSFCell cell, BaseXSSFEvaluationWorkbook evalWb){
 
         CTCellFormula f = cell.getCTCell().getF();
         if (f != null && f.getT() == STCellFormulaType.SHARED && f.isSetRef() && f.getStringValue() != null) {
@@ -4634,9 +4636,9 @@ public class XSSFSheet extends POIXMLDoc
                     XSSFRow row = getRow(i);
                     if(row != null) for(int j = cell.getColumnIndex(); j <= ref.getLastColumn(); j++){
                         XSSFCell nextCell = row.getCell(j);
-                        if(nextCell != null && nextCell != cell){
+                        if(nextCell != null && nextCell != cell && nextCell.getCellType() == CellType.FORMULA){
                             CTCellFormula nextF = nextCell.getCTCell().getF();
-                            nextF.setStringValue(nextCell.getCellFormula());
+                            nextF.setStringValue(nextCell.getCellFormula(evalWb));
                             CellRangeAddress nextRef = new CellRangeAddress(
                                     nextCell.getRowIndex(), ref.getLastRow(),
                                     nextCell.getColumnIndex(), ref.getLastColumn());

Modified: poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java?rev=1844295&r1=1844294&r2=1844295&view=diff
==============================================================================
--- poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java (original)
+++ poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java Fri Oct 19 01:11:47 2018
@@ -17,14 +17,24 @@
 
 package org.apache.poi.xssf.usermodel;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.fail;
 
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
-import org.apache.poi.ss.usermodel.*;
+import org.apache.poi.ss.usermodel.BaseTestFormulaEvaluator;
+import org.apache.poi.ss.usermodel.Cell;
+import org.apache.poi.ss.usermodel.CellValue;
+import org.apache.poi.ss.usermodel.CreationHelper;
+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.ss.util.CellReference;
 import org.apache.poi.xssf.XSSFITestDataProvider;
 import org.apache.poi.xssf.XSSFTestDataSamples;
@@ -437,6 +447,10 @@ public final class TestXSSFFormulaEvalua
         assertEquals("D 0,068", evaluator.evaluate(wb.getSheetAt(0).getRow(1).getCell(1)));
     }
 
+    /**
+     * see bug 62275
+     * @throws IOException
+     */
     @Test
     public void testBug62275() throws IOException {
         try (Workbook wb = new XSSFWorkbook()) {
@@ -451,4 +465,29 @@ public final class TestXSSFFormulaEvalua
             eval.evaluate(cell);
         }
     }
+    
+    /**
+     * see bug 62834, handle when a shared formula range doesn't contain only formula cells
+     * @throws IOException 
+     */
+    @Test
+    public void testBug62834() throws IOException {
+        try (Workbook wb = XSSFTestDataSamples.openSampleWorkbook("62834.xlsx")) {
+            FormulaEvaluator evaluator = wb.getCreationHelper().createFormulaEvaluator();
+
+            Cell a2 = wb.getSheetAt(0).getRow(1).getCell(0);
+            Cell value = evaluator.evaluateInCell(a2);
+            assertEquals("wrong value A2", "a value", value.getStringCellValue());
+            
+//            evaluator.clearAllCachedResultValues();
+            
+            Cell a3 = wb.getSheetAt(0).getRow(2).getCell(0);
+            value = evaluator.evaluateInCell(a3);
+            assertEquals("wrong value A3", "a value", value.getStringCellValue());
+            
+            Cell a5 = wb.getSheetAt(0).getRow(4).getCell(0);
+            value = evaluator.evaluateInCell(a5);
+            assertEquals("wrong value A5", "another value", value.getStringCellValue());
+        }
+    }
 }

Added: poi/trunk/test-data/spreadsheet/62834.xlsx
URL: http://svn.apache.org/viewvc/poi/trunk/test-data/spreadsheet/62834.xlsx?rev=1844295&view=auto
==============================================================================
Binary file - no diff available.

Propchange: poi/trunk/test-data/spreadsheet/62834.xlsx
------------------------------------------------------------------------------
--- svn:mime-type (added)
+++ svn:mime-type Fri Oct 19 01:11:47 2018
@@ -0,0 +1 @@
+application/vnd.openxmlformats-officedocument.spreadsheetml.sheet



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