From 59861b7836b9d5909ea95a187f6be09afe8604f0 Mon Sep 17 00:00:00 2001
From: Nick Burch <nick@apache.org>
Date: Fri, 25 Jul 2014 16:42:31 +0000
Subject: [PATCH] Complete evaluation support for multi-sheet references for
 bug #55906

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1613467 13f79535-47bb-0310-9956-ffa450edef68
---
 .../poi/ss/formula/functions/CountUtils.java  | 52 +++++++++++--------
 .../poi/ss/formula/functions/Countblank.java  |  6 +--
 .../poi/ss/formula/functions/Countif.java     |  6 +--
 .../MultiOperandNumericFunction.java          | 18 ++++++-
 .../usermodel/TestXSSFFormulaEvaluation.java  |  8 ++-
 .../formula/atp/TestNetworkdaysFunction.java  |  6 ++-
 .../ss/formula/atp/TestWorkdayFunction.java   |  6 ++-
 .../poi/ss/formula/eval/TestRangeEval.java    |  3 ++
 .../poi/ss/formula/functions/EvalFactory.java |  3 ++
 9 files changed, 70 insertions(+), 38 deletions(-)

diff --git a/src/java/org/apache/poi/ss/formula/functions/CountUtils.java b/src/java/org/apache/poi/ss/formula/functions/CountUtils.java
index 6c5fbce7dc..270f787bc7 100644
--- a/src/java/org/apache/poi/ss/formula/functions/CountUtils.java
+++ b/src/java/org/apache/poi/ss/formula/functions/CountUtils.java
@@ -17,6 +17,7 @@
 
 package org.apache.poi.ss.formula.functions;
 
+import org.apache.poi.ss.formula.ThreeDEval;
 import org.apache.poi.ss.formula.TwoDEval;
 import org.apache.poi.ss.formula.eval.RefEval;
 import org.apache.poi.ss.formula.eval.ValueEval;
@@ -42,30 +43,32 @@ final class CountUtils {
         boolean matches(TwoDEval x, int rowIndex, int columnIndex);
     }
 
-	/**
-	 * @return the number of evaluated cells in the range that match the specified criteria
-	 */
-	public static int countMatchingCellsInArea(TwoDEval areaEval, I_MatchPredicate criteriaPredicate) {
-		int result = 0;
-
-		int height = areaEval.getHeight();
-		int width = areaEval.getWidth();
-		for (int rrIx=0; rrIx<height; rrIx++) {
-			for (int rcIx=0; rcIx<width; rcIx++) {
-				ValueEval ve = areaEval.getValue(rrIx, rcIx);
+    /**
+     * @return the number of evaluated cells in the range that match the specified criteria
+     */
+    public static int countMatchingCellsInArea(ThreeDEval areaEval, I_MatchPredicate criteriaPredicate) {
+        int result = 0;
 
-                if(criteriaPredicate instanceof I_MatchAreaPredicate){
-                    I_MatchAreaPredicate areaPredicate = (I_MatchAreaPredicate)criteriaPredicate;
-                    if(!areaPredicate.matches(areaEval, rrIx, rcIx)) continue;
+        for (int sIx=areaEval.getFirstSheetIndex(); sIx <= areaEval.getLastSheetIndex(); sIx++) {
+            int height = areaEval.getHeight();
+            int width = areaEval.getWidth();
+            for (int rrIx=0; rrIx<height; rrIx++) {
+                for (int rcIx=0; rcIx<width; rcIx++) {
+                    ValueEval ve = areaEval.getValue(sIx, rrIx, rcIx);
+    
+                    if(criteriaPredicate instanceof I_MatchAreaPredicate){
+                        I_MatchAreaPredicate areaPredicate = (I_MatchAreaPredicate)criteriaPredicate;
+                        if(!areaPredicate.matches(areaEval, rrIx, rcIx)) continue;
+                    }
+    
+                    if(criteriaPredicate.matches(ve)) {
+                        result++;
+                    }
                 }
-
-				if(criteriaPredicate.matches(ve)) {
-					result++;
-				}
-			}
-		}
-		return result;
-	}
+            }
+        }
+        return result;
+    }
 	/**
      * @return the number of evaluated cells in the range that match the specified criteria
 	 */
@@ -84,8 +87,11 @@ final class CountUtils {
 		if (eval == null) {
 			throw new IllegalArgumentException("eval must not be null");
 		}
+        if (eval instanceof ThreeDEval) {
+            return countMatchingCellsInArea((ThreeDEval) eval, criteriaPredicate);
+        }
 		if (eval instanceof TwoDEval) {
-			return countMatchingCellsInArea((TwoDEval) eval, criteriaPredicate);
+		    throw new IllegalArgumentException("Count requires 3D Evals, 2D ones aren't supported");
 		}
 		if (eval instanceof RefEval) {
 			return CountUtils.countMatchingCellsInRef((RefEval) eval, criteriaPredicate);
diff --git a/src/java/org/apache/poi/ss/formula/functions/Countblank.java b/src/java/org/apache/poi/ss/formula/functions/Countblank.java
index 522e185a6c..5b83e4fd31 100644
--- a/src/java/org/apache/poi/ss/formula/functions/Countblank.java
+++ b/src/java/org/apache/poi/ss/formula/functions/Countblank.java
@@ -17,7 +17,7 @@
 
 package org.apache.poi.ss.formula.functions;
 
-import org.apache.poi.ss.formula.TwoDEval;
+import org.apache.poi.ss.formula.ThreeDEval;
 import org.apache.poi.ss.formula.eval.BlankEval;
 import org.apache.poi.ss.formula.eval.NumberEval;
 import org.apache.poi.ss.formula.eval.RefEval;
@@ -42,8 +42,8 @@ public final class Countblank extends Fixed1ArgFunction {
 		double result;
 		if (arg0 instanceof RefEval) {
 			result = CountUtils.countMatchingCellsInRef((RefEval) arg0, predicate);
-		} else if (arg0 instanceof TwoDEval) {
-			result = CountUtils.countMatchingCellsInArea((TwoDEval) arg0, predicate);
+		} else if (arg0 instanceof ThreeDEval) {
+			result = CountUtils.countMatchingCellsInArea((ThreeDEval) arg0, predicate);
 		} else {
 			throw new IllegalArgumentException("Bad range arg type (" + arg0.getClass().getName() + ")");
 		}
diff --git a/src/java/org/apache/poi/ss/formula/functions/Countif.java b/src/java/org/apache/poi/ss/formula/functions/Countif.java
index ce73b8563f..6f27fdf48e 100644
--- a/src/java/org/apache/poi/ss/formula/functions/Countif.java
+++ b/src/java/org/apache/poi/ss/formula/functions/Countif.java
@@ -19,7 +19,7 @@ package org.apache.poi.ss.formula.functions;
 
 import java.util.regex.Pattern;
 
-import org.apache.poi.ss.formula.TwoDEval;
+import org.apache.poi.ss.formula.ThreeDEval;
 import org.apache.poi.ss.formula.eval.BlankEval;
 import org.apache.poi.ss.formula.eval.BoolEval;
 import org.apache.poi.ss.formula.eval.ErrorEval;
@@ -445,8 +445,8 @@ public final class Countif extends Fixed2ArgFunction {
 
 		if (rangeArg instanceof RefEval) {
 			return CountUtils.countMatchingCellsInRef((RefEval) rangeArg, criteriaPredicate);
-		} else if (rangeArg instanceof TwoDEval) {
-			return CountUtils.countMatchingCellsInArea((TwoDEval) rangeArg, criteriaPredicate);
+		} else if (rangeArg instanceof ThreeDEval) {
+			return CountUtils.countMatchingCellsInArea((ThreeDEval) rangeArg, criteriaPredicate);
 		} else {
 			throw new IllegalArgumentException("Bad range arg type (" + rangeArg.getClass().getName() + ")");
 		}
diff --git a/src/java/org/apache/poi/ss/formula/functions/MultiOperandNumericFunction.java b/src/java/org/apache/poi/ss/formula/functions/MultiOperandNumericFunction.java
index 528fa9bf4a..6e0f364f1d 100644
--- a/src/java/org/apache/poi/ss/formula/functions/MultiOperandNumericFunction.java
+++ b/src/java/org/apache/poi/ss/formula/functions/MultiOperandNumericFunction.java
@@ -17,6 +17,7 @@
 
 package org.apache.poi.ss.formula.functions;
 
+import org.apache.poi.ss.formula.ThreeDEval;
 import org.apache.poi.ss.formula.TwoDEval;
 import org.apache.poi.ss.formula.eval.BlankEval;
 import org.apache.poi.ss.formula.eval.BoolEval;
@@ -30,7 +31,6 @@ import org.apache.poi.ss.formula.eval.StringValueEval;
 import org.apache.poi.ss.formula.eval.ValueEval;
 
 /**
- * @author Amol S. Deshmukh &lt; amolweb at ya hoo dot com &gt;
  * This is the super class for all excel function evaluator
  * classes that take variable number of operands, and
  * where the order of operands does not matter
@@ -141,7 +141,21 @@ public abstract class MultiOperandNumericFunction implements Function {
 	 * Collects values from a single argument
 	 */
 	private void collectValues(ValueEval operand, DoubleList temp) throws EvaluationException {
-
+        if (operand instanceof ThreeDEval) {
+            ThreeDEval ae = (ThreeDEval) operand;
+            for (int sIx=ae.getFirstSheetIndex(); sIx <= ae.getLastSheetIndex(); sIx++) {
+                int width = ae.getWidth();
+                int height = ae.getHeight();
+                for (int rrIx=0; rrIx<height; rrIx++) {
+                    for (int rcIx=0; rcIx<width; rcIx++) {
+                        ValueEval ve = ae.getValue(sIx, rrIx, rcIx);
+                        if(!isSubtotalCounted() && ae.isSubTotal(rrIx, rcIx)) continue;
+                        collectValue(ve, true, temp);
+                    }
+                }
+            }
+            return;
+        }
 		if (operand instanceof TwoDEval) {
 			TwoDEval ae = (TwoDEval) operand;
 			int width = ae.getWidth();
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 09c45f0404..aaba1643d6 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java
@@ -247,10 +247,8 @@ public final class TestXSSFFormulaEvaluation extends BaseTestFormulaEvaluator {
      *  from Sheets 1 through Sheet 3).
      * This test, based on common test files for HSSF and XSSF, checks
      *  that we can correctly evaluate these
-     * 
-     * DISABLED pending support, see bug #55906
      */
-    public void DISABLEDtestMultiSheetAreasHSSFandXSSF() throws Exception {
+    public void testMultiSheetAreasHSSFandXSSF() throws Exception {
         Workbook[] wbs = new Workbook[] {
                 HSSFTestDataSamples.openSampleWorkbook("55906-MultiSheetRefs.xls"),
                 XSSFTestDataSamples.openSampleWorkbook("55906-MultiSheetRefs.xlsx")
@@ -264,11 +262,11 @@ public final class TestXSSFFormulaEvaluation extends BaseTestFormulaEvaluator {
             Cell sumFA = s1.getRow(2).getCell(7);
             assertNotNull(sumFA);
             assertEquals("SUM(Sheet1:Sheet3!A1:B2)", sumFA.getCellFormula());
-            assertEquals("110.0", evaluator.evaluate(sumFA).formatAsString());
+            assertEquals("Failed for " + wb.getClass(), "110.0", evaluator.evaluate(sumFA).formatAsString());
 
             
             // Various Stats formulas on ranges of numbers
-            Cell avgFA = s1.getRow(2).getCell(7);
+            Cell avgFA = s1.getRow(2).getCell(8);
             assertNotNull(avgFA);
             assertEquals("AVERAGE(Sheet1:Sheet3!A1:B2)", avgFA.getCellFormula());
             assertEquals("27.5", evaluator.evaluate(avgFA).formatAsString());
diff --git a/src/testcases/org/apache/poi/ss/formula/atp/TestNetworkdaysFunction.java b/src/testcases/org/apache/poi/ss/formula/atp/TestNetworkdaysFunction.java
index 594926fe24..15309d5c19 100644
--- a/src/testcases/org/apache/poi/ss/formula/atp/TestNetworkdaysFunction.java
+++ b/src/testcases/org/apache/poi/ss/formula/atp/TestNetworkdaysFunction.java
@@ -116,9 +116,13 @@ public class TestNetworkdaysFunction extends TestCase {
         }
 
         @Override
-        public ValueEval getRelativeValue(int relativeRowIndex, int relativeColumnIndex) {
+        public ValueEval getRelativeValue(int sheetIndex, int relativeRowIndex, int relativeColumnIndex) {
             return this.holidays.get(relativeColumnIndex);
         }
+        @Override
+        public ValueEval getRelativeValue(int relativeRowIndex, int relativeColumnIndex) {
+            return getRelativeValue(-1, relativeRowIndex, relativeColumnIndex);
+        }
 
         public AreaEval offset(int relFirstRowIx, int relLastRowIx, int relFirstColIx, int relLastColIx) {
             return null;
diff --git a/src/testcases/org/apache/poi/ss/formula/atp/TestWorkdayFunction.java b/src/testcases/org/apache/poi/ss/formula/atp/TestWorkdayFunction.java
index 77f2d9ed9c..6deb235cd1 100644
--- a/src/testcases/org/apache/poi/ss/formula/atp/TestWorkdayFunction.java
+++ b/src/testcases/org/apache/poi/ss/formula/atp/TestWorkdayFunction.java
@@ -159,9 +159,13 @@ public class TestWorkdayFunction extends TestCase {
         }
 
         @Override
-        public ValueEval getRelativeValue(int relativeRowIndex, int relativeColumnIndex) {
+        public ValueEval getRelativeValue(int sheetIndex, int relativeRowIndex, int relativeColumnIndex) {
             return this.holidays.get(relativeColumnIndex);
         }
+        @Override
+        public ValueEval getRelativeValue(int relativeRowIndex, int relativeColumnIndex) {
+            return getRelativeValue(-1, relativeRowIndex, relativeColumnIndex);
+        }
 
         public AreaEval offset(int relFirstRowIx, int relLastRowIx, int relFirstColIx, int relLastColIx) {
             return null;
diff --git a/src/testcases/org/apache/poi/ss/formula/eval/TestRangeEval.java b/src/testcases/org/apache/poi/ss/formula/eval/TestRangeEval.java
index 67cbcb2f7a..5057c41150 100644
--- a/src/testcases/org/apache/poi/ss/formula/eval/TestRangeEval.java
+++ b/src/testcases/org/apache/poi/ss/formula/eval/TestRangeEval.java
@@ -96,6 +96,9 @@ public final class TestRangeEval extends TestCase {
 		public ValueEval getRelativeValue(int relativeRowIndex, int relativeColumnIndex) {
 			throw new RuntimeException("not expected to be called during this test");
 		}
+        public ValueEval getRelativeValue(int sheetIndex, int relativeRowIndex, int relativeColumnIndex) {
+            throw new RuntimeException("not expected to be called during this test");
+        }
 		public AreaEval offset(int relFirstRowIx, int relLastRowIx, int relFirstColIx,
 				int relLastColIx) {
 			AreaI area = new OffsetArea(getFirstRow(), getFirstColumn(),
diff --git a/src/testcases/org/apache/poi/ss/formula/functions/EvalFactory.java b/src/testcases/org/apache/poi/ss/formula/functions/EvalFactory.java
index e294be59f3..4d755ebabd 100644
--- a/src/testcases/org/apache/poi/ss/formula/functions/EvalFactory.java
+++ b/src/testcases/org/apache/poi/ss/formula/functions/EvalFactory.java
@@ -89,6 +89,9 @@ public final class EvalFactory {
 			_values = values;
 		}
 		public ValueEval getRelativeValue(int relativeRowIndex, int relativeColumnIndex) {
+		    return getRelativeValue(-1, relativeRowIndex, relativeColumnIndex);
+		}
+        public ValueEval getRelativeValue(int sheetIndex, int relativeRowIndex, int relativeColumnIndex) {
 			if (relativeRowIndex < 0 || relativeRowIndex >=getHeight()) {
 				throw new IllegalArgumentException("row index out of range");
 			}
-- 
2.39.5