Get basic SXSSF formula evaluation working, for cells/references in the window #58200

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

8 files changed, 89 insertions, 44 deletions

diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java
index 667eadcf9b..c5d809cada 100644
--- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java
+++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationSheet.java
@@ -36,6 +36,9 @@ final class SXSSFEvaluationSheet implements EvaluationSheet {
     public EvaluationCell getCell(int rowIndex, int columnIndex) {
         SXSSFRow row = _xs.getRow(rowIndex);
         if (row == null) {
+            if (rowIndex <= _xs.getLastFlushedRowNum()) {
+                throw new SXSSFFormulaEvaluator.RowFlushedException(rowIndex);
+            }
             return null;
         }
         SXSSFCell cell = row.getCell(columnIndex);
diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationWorkbook.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationWorkbook.java
index ad6c9de3dc..b873588db8 100644
--- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationWorkbook.java
+++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFEvaluationWorkbook.java
@@ -17,7 +17,11 @@
 
 package org.apache.poi.xssf.streaming;
 
+import org.apache.poi.ss.formula.EvaluationCell;
 import org.apache.poi.ss.formula.EvaluationSheet;
+import org.apache.poi.ss.formula.FormulaParser;
+import org.apache.poi.ss.formula.FormulaType;
+import org.apache.poi.ss.formula.ptg.Ptg;
 import org.apache.poi.xssf.usermodel.BaseXSSFEvaluationWorkbook;
 
 /**
@@ -46,4 +50,10 @@ public final class SXSSFEvaluationWorkbook extends BaseXSSFEvaluationWorkbook {
     public EvaluationSheet getSheet(int sheetIndex) {
         return new SXSSFEvaluationSheet(_uBook.getSheetAt(sheetIndex));
     }
+    
+    public Ptg[] getFormulaTokens(EvaluationCell evalCell) {
+        SXSSFCell cell = ((SXSSFEvaluationCell)evalCell).getSXSSFCell();
+        SXSSFEvaluationWorkbook frBook = SXSSFEvaluationWorkbook.create(_uBook);
+        return FormulaParser.parse(cell.getCellFormula(), frBook, FormulaType.CELL, _uBook.getSheetIndex(cell.getSheet()));
+    }
 }

diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFFormulaEvaluator.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFFormulaEvaluator.java
index d1c82ba244..40abe9fe8d 100644
--- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFFormulaEvaluator.java
+++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFFormulaEvaluator.java
@@ -18,6 +18,9 @@
 package org.apache.poi.xssf.streaming;
 
 import org.apache.poi.ss.formula.EvaluationCell;
+import org.apache.poi.ss.formula.IStabilityClassifier;
+import org.apache.poi.ss.formula.WorkbookEvaluator;
+import org.apache.poi.ss.formula.udf.UDFFinder;
 import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.ss.usermodel.Row;
 import org.apache.poi.util.POILogFactory;
@@ -34,11 +37,27 @@ public class SXSSFFormulaEvaluator extends XSSFFormulaEvaluator {
     private SXSSFWorkbook wb;
     
     public SXSSFFormulaEvaluator(SXSSFWorkbook workbook) {
-        super(workbook.getXSSFWorkbook());
+        this(workbook, null, null);
+    }
+    private SXSSFFormulaEvaluator(SXSSFWorkbook workbook, IStabilityClassifier stabilityClassifier, UDFFinder udfFinder) {
+        this(workbook, new WorkbookEvaluator(SXSSFEvaluationWorkbook.create(workbook), stabilityClassifier, udfFinder));
+    }
+    private SXSSFFormulaEvaluator(SXSSFWorkbook workbook, WorkbookEvaluator bookEvaluator) {
+        super(workbook.getXSSFWorkbook(), bookEvaluator);
         this.wb = workbook;
     }
     
     /**
+     * @param stabilityClassifier used to optimise caching performance. Pass <code>null</code>
+     * for the (conservative) assumption that any cell may have its definition changed after
+     * evaluation begins.
+     * @param udfFinder pass <code>null</code> for default (AnalysisToolPak only)
+     */
+    public static SXSSFFormulaEvaluator create(SXSSFWorkbook workbook, IStabilityClassifier stabilityClassifier, UDFFinder udfFinder) {
+        return new SXSSFFormulaEvaluator(workbook, stabilityClassifier, udfFinder);
+    }
+    
+    /**
      * Turns a SXSSFCell into a SXSSFEvaluationCell
      */
     @Override
@@ -63,7 +82,7 @@ public class SXSSFFormulaEvaluator extends XSSFFormulaEvaluator {
         // Check they're all available
         for (int i=0; i<wb.getNumberOfSheets(); i++) {
             SXSSFSheet s = wb.getSheetAt(i);
-            if (s.isFlushed()) {
+            if (s.areAllRowsFlushed()) {
                 throw new SheetsFlushedException();
             }
         }
@@ -73,12 +92,10 @@ public class SXSSFFormulaEvaluator extends XSSFFormulaEvaluator {
             SXSSFSheet s = wb.getSheetAt(i);
             
             // Check if any rows have already been flushed out
-            int firstRowNum = s.getFirstRowNum();
-            int firstAvailableRowNum = s.iterator().next().getRowNum();
-            if (firstRowNum != firstAvailableRowNum) {
-                if (skipOutOfWindow) throw new RowsFlushedException();
-                logger.log(POILogger.INFO, "Rows from " + firstRowNum + " to" +
-                           (firstAvailableRowNum-1) + " have already been flushed, skipping");
+            int lastFlushedRowNum = s.getLastFlushedRowNum();
+            if (lastFlushedRowNum > -1) {
+                if (! skipOutOfWindow) throw new RowFlushedException(0);
+                logger.log(POILogger.INFO, "Rows up to " + lastFlushedRowNum + " have already been flushed, skipping");
             }
             
             // Evaluate what we have
@@ -109,9 +126,9 @@ public class SXSSFFormulaEvaluator extends XSSFFormulaEvaluator {
             super("One or more sheets have been flushed, cannot evaluate all cells");
         }
     }
-    public static class RowsFlushedException extends IllegalStateException {
-        protected RowsFlushedException() {
-            super("One or more rows have been flushed, cannot evaluate all cells");
+    public static class RowFlushedException extends IllegalStateException {
+        protected RowFlushedException(int rowNum) {
+            super("Row " + rowNum + " has been flushed, cannot evaluate all cells");
         }
     }
 }
diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java
index 2d66c76c8c..283b829c2a 100644
--- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java
+++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java
@@ -59,7 +59,8 @@ public class SXSSFSheet implements Sheet, Cloneable
     private SheetDataWriter _writer;
     private int _randomAccessWindowSize = SXSSFWorkbook.DEFAULT_WINDOW_SIZE;
     private int outlineLevelRow = 0;
-    private boolean flushed = false;
+    private int lastFlushedRowNumber = -1;
+    private boolean allFlushed = false;
 
     public SXSSFSheet(SXSSFWorkbook workbook, XSSFSheet xSheet) throws IOException {
         _workbook = workbook;
@@ -135,7 +136,7 @@ public class SXSSFSheet implements Sheet, Cloneable
             initialAllocationSize=10;
         SXSSFRow newRow=new SXSSFRow(this,initialAllocationSize);
         _rows.put(new Integer(rownum),newRow);
-        flushed = false;
+        allFlushed = false;
         if(_randomAccessWindowSize>=0&&_rows.size()>_randomAccessWindowSize)
         {
             try
@@ -1211,7 +1212,7 @@ public class SXSSFSheet implements Sheet, Cloneable
      * @param rowIndex the zero based row index to collapse
      */
     private void collapseRow(int rowIndex) {
-        SXSSFRow row = (SXSSFRow) getRow(rowIndex);
+        SXSSFRow row = getRow(rowIndex);
         if(row == null) {
 			throw new IllegalArgumentException("Invalid row number("+ rowIndex + "). Row does not exist.");
 		} else {
@@ -1219,7 +1220,7 @@ public class SXSSFSheet implements Sheet, Cloneable
 
             // Hide all the columns until the end of the group
             int lastRow = writeHidden(row, startRow, true);
-            SXSSFRow lastRowObj = (SXSSFRow) getRow(lastRow); 
+            SXSSFRow lastRowObj = getRow(lastRow); 
             if (lastRowObj != null) {
                 lastRowObj.setCollapsed(true);
             } else {
@@ -1241,7 +1242,7 @@ public class SXSSFSheet implements Sheet, Cloneable
         }
         int currentRow = rowIndex;
         while (getRow(currentRow) != null) {
-            if (((SXSSFRow) getRow(currentRow)).getOutlineLevel() < level)
+            if (getRow(currentRow).getOutlineLevel() < level)
                 return currentRow + 1;
             currentRow--;
         }
@@ -1250,12 +1251,12 @@ public class SXSSFSheet implements Sheet, Cloneable
     
     private int writeHidden(SXSSFRow xRow, int rowIndex, boolean hidden) {
         int level = xRow.getOutlineLevel();
-        SXSSFRow currRow = (SXSSFRow) getRow(rowIndex);
+        SXSSFRow currRow = getRow(rowIndex);
 
         while (currRow != null && currRow.getOutlineLevel() >= level) {
             currRow.setHidden(hidden);
             rowIndex++;
-            currRow = (SXSSFRow) getRow(rowIndex);
+            currRow = getRow(rowIndex);
         }
         return rowIndex;
     }
@@ -1466,8 +1467,14 @@ public class SXSSFSheet implements Sheet, Cloneable
     /**
      * Are all rows flushed to disk?
      */
-    public boolean isFlushed() {
-        return flushed;
+    public boolean areAllRowsFlushed() {
+        return allFlushed;
+    }
+    /**
+     * @return Last row number to be flushed to disk, or -1 if none flushed yet
+     */
+    public int getLastFlushedRowNum() {
+        return lastFlushedRowNumber;
     }
 
     /**
@@ -1478,7 +1485,7 @@ public class SXSSFSheet implements Sheet, Cloneable
     public void flushRows(int remaining) throws IOException
     {
         while(_rows.size() > remaining) flushOneRow();
-        if (remaining == 0) flushed = true;
+        if (remaining == 0) allFlushed = true;
     }
 
     /**
@@ -1499,6 +1506,7 @@ public class SXSSFSheet implements Sheet, Cloneable
             SXSSFRow row = _rows.get(firstRowNum);
             _writer.writeRow(rowIndex, row);
             _rows.remove(firstRowNum);
+            lastFlushedRowNumber = rowIndex;
         }
     }
     public void changeRowNum(SXSSFRow row, int newRowNum)
@@ -1524,7 +1532,7 @@ public class SXSSFSheet implements Sheet, Cloneable
      * @return true if the file was deleted, false if it wasn't.
      */
     boolean dispose() throws IOException {
-        if (!flushed) flushRows();
+        if (!allFlushed) flushRows();
         return _writer.dispose();
     }
 
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFEvaluationWorkbook.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFEvaluationWorkbook.java
index 7776f5f0f9..a1c31b184e 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFEvaluationWorkbook.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/BaseXSSFEvaluationWorkbook.java
@@ -292,11 +292,6 @@ public abstract class BaseXSSFEvaluationWorkbook implements FormulaRenderingWork
 		int ix = namePtg.getIndex();
 		return new Name(_uBook.getNameAt(ix), ix, this);
 	}
-	public Ptg[] getFormulaTokens(EvaluationCell evalCell) {
-		XSSFCell cell = ((XSSFEvaluationCell)evalCell).getXSSFCell();
-		XSSFEvaluationWorkbook frBook = XSSFEvaluationWorkbook.create(_uBook);
-		return FormulaParser.parse(cell.getCellFormula(), frBook, FormulaType.CELL, _uBook.getSheetIndex(cell.getSheet()));
-	}
 	
     public UDFFinder getUDFFinder(){
         return _uBook.getUDFFinder();
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationWorkbook.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationWorkbook.java
index 49b30ab87d..70fc720671 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationWorkbook.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFEvaluationWorkbook.java
@@ -17,7 +17,11 @@
 
 package org.apache.poi.xssf.usermodel;
 
+import org.apache.poi.ss.formula.EvaluationCell;
 import org.apache.poi.ss.formula.EvaluationSheet;
+import org.apache.poi.ss.formula.FormulaParser;
+import org.apache.poi.ss.formula.FormulaType;
+import org.apache.poi.ss.formula.ptg.Ptg;
 
 /**
  * Internal POI use only
@@ -42,4 +46,10 @@ public final class XSSFEvaluationWorkbook extends BaseXSSFEvaluationWorkbook {
 	public EvaluationSheet getSheet(int sheetIndex) {
 		return new XSSFEvaluationSheet(_uBook.getSheetAt(sheetIndex));
 	}
+	
+    public Ptg[] getFormulaTokens(EvaluationCell evalCell) {
+        XSSFCell cell = ((XSSFEvaluationCell)evalCell).getXSSFCell();
+        XSSFEvaluationWorkbook frBook = XSSFEvaluationWorkbook.create(_uBook);
+        return FormulaParser.parse(cell.getCellFormula(), frBook, FormulaType.CELL, _uBook.getSheetIndex(cell.getSheet()));
+    }
 }
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 e9eaf77c01..e3cba3caf7 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFormulaEvaluator.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFFormulaEvaluator.java
@@ -51,7 +51,10 @@ public class XSSFFormulaEvaluator implements FormulaEvaluator, WorkbookEvaluator
         this(workbook, null, null);
     }
     private XSSFFormulaEvaluator(XSSFWorkbook workbook, IStabilityClassifier stabilityClassifier, UDFFinder udfFinder) {
-        _bookEvaluator = new WorkbookEvaluator(XSSFEvaluationWorkbook.create(workbook), stabilityClassifier, udfFinder);
+        this(workbook, new WorkbookEvaluator(XSSFEvaluationWorkbook.create(workbook), stabilityClassifier, udfFinder));
+    }
+    protected XSSFFormulaEvaluator(XSSFWorkbook workbook, WorkbookEvaluator bookEvaluator) {
+        _bookEvaluator = bookEvaluator;
         _book = workbook;
     }
 
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFFormulaEvaluation.java b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFFormulaEvaluation.java
index 5c1ec3dd28..1c750fedbf 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFFormulaEvaluation.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFFormulaEvaluation.java
@@ -28,7 +28,6 @@ import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.ss.usermodel.FormulaEvaluator;
 import org.apache.poi.xssf.SXSSFITestDataProvider;
 import org.apache.poi.xssf.usermodel.XSSFWorkbook;
-import org.junit.Ignore;
 import org.junit.Test;
 
 /**
@@ -38,7 +37,6 @@ import org.junit.Test;
  *  cell is in the current window, and all references
  *  from the cell are in the current window
  */
-@Ignore
 public final class TestSXSSFFormulaEvaluation {
     public static final SXSSFITestDataProvider _testDataProvider = SXSSFITestDataProvider.instance;
 
@@ -54,24 +52,18 @@ public final class TestSXSSFFormulaEvaluation {
         
         FormulaEvaluator eval = wb.getCreationHelper().createFormulaEvaluator();
         
-        // References outside window will fail
         s.createRow(0).createCell(0).setCellFormula("1+2");
         s.createRow(1).createCell(0).setCellFormula("A21");
-        try {
-            eval.evaluateAll();
-            fail("Evaluate All shouldn't work, as references outside the window");
-        } catch(Exception e) {
-            System.err.println(e); // TODO
-        }
+        for (int i=2; i<19; i++) { s.createRow(i); }
         
-        // Cells outside window will fail
-        s.createRow(10).createCell(0).setCellFormula("A1+A2");
+        // Cells outside window will fail, whether referenced or not
+        s.createRow(19).createCell(0).setCellFormula("A1+A2");
         s.createRow(20).createCell(0).setCellFormula("A1+A11+100");
         try {
             eval.evaluateAll();
             fail("Evaluate All shouldn't work, as some cells outside the window");
-        } catch(Exception e) {
-            System.err.println(e); // TODO
+        } catch(SXSSFFormulaEvaluator.RowFlushedException e) {
+            // Expected
         }
         
         
@@ -97,16 +89,24 @@ public final class TestSXSSFFormulaEvaluation {
     public void testEvaluateRefOutsideWindowFails() {
         SXSSFWorkbook wb = new SXSSFWorkbook(5);
         SXSSFSheet s = wb.createSheet();
+        
         s.createRow(0).createCell(0).setCellFormula("1+2");
+        assertEquals(false, s.areAllRowsFlushed());
+        assertEquals(-1, s.getLastFlushedRowNum());
+        
+        for (int i=1; i<=19; i++) { s.createRow(i); }
         Cell c = s.createRow(20).createCell(0);
         c.setCellFormula("A1+100");
         
+        assertEquals(false, s.areAllRowsFlushed());
+        assertEquals(15, s.getLastFlushedRowNum());
+        
         FormulaEvaluator eval = wb.getCreationHelper().createFormulaEvaluator();
         try {
             eval.evaluateFormulaCell(c);
             fail("Evaluate shouldn't work, as reference outside the window");
-        } catch(Exception e) {
-            System.err.println(e); // TODO
+        } catch(SXSSFFormulaEvaluator.RowFlushedException e) {
+            // Expected
         }
     }
     
@@ -163,7 +163,6 @@ public final class TestSXSSFFormulaEvaluation {
         
         c = s.createRow(1).createCell(0);
         c.setCellFormula("CONCATENATE(\"hello\",\" \",\"world\")");
-        assertEquals("", c.getStringCellValue());
         eval.evaluateFormulaCell(c);
         assertEquals("hello world", c.getStringCellValue());
     }