Author: gwoolsey
Date: Wed Jul 26 22:19:58 2017
New Revision: 1803121
URL: http://svn.apache.org/viewvc?rev=1803121&view=rev
Log:
Fix data validation value list evaluation
One of my users found that my initial implementation was lacking a core
distinction - most evaluations expect a single result, and "unwrap" 2/3D
ValueEval results to a single value based on the input row/column.
However, data validation list formulas explicitly are expected to return a 2D
ValueEval. This worked when the formula was simple and evaluated to a single
Ptg, but only returned one value when the formula was more complex, or
referenced a named range defined as a complex formula.
This change teaches WorkbookEvaluator about the distinction, by way of a new
attribute for FormulaType.
There is room for discussion over how it is implemented, but this works for me.
Includes the failing workbook we had as a new unit test.
While I was in FormulaType I went ahead and removed the deprecated, unused, and
redundant code marked for removal in 3.17.
Added:
poi/trunk/test-data/spreadsheet/dataValidationTableRange.xlsx (with props)
Modified:
poi/trunk/src/java/org/apache/poi/ss/formula/DataValidationEvaluator.java
poi/trunk/src/java/org/apache/poi/ss/formula/FormulaType.java
poi/trunk/src/java/org/apache/poi/ss/formula/OperationEvaluationContext.java
poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataValidation.java
Modified:
poi/trunk/src/java/org/apache/poi/ss/formula/DataValidationEvaluator.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/DataValidationEvaluator.java?rev=1803121&r1=1803120&r2=1803121&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/DataValidationEvaluator.java
(original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/DataValidationEvaluator.java
Wed Jul 26 22:19:58 2017
@@ -196,7 +196,8 @@ public class DataValidationEvaluator {
}
} else if (formula != null) {
// evaluate formula for cell refs then get their values
- ValueEval eval =
context.getEvaluator().getWorkbookEvaluator().evaluate(formula,
context.getTarget(), context.getRegion());
+ // note this should return the raw formula result, not the
"unwrapped" version that returns a single value.
+ ValueEval eval =
context.getEvaluator().getWorkbookEvaluator().evaluateList(formula,
context.getTarget(), context.getRegion());
// formula is a StringEval if the validation is by a fixed list.
Use the explicit list later.
// there is no way from the model to tell if the list is fixed
values or formula based.
if (eval instanceof TwoDEval) {
@@ -344,6 +345,7 @@ public class DataValidationEvaluator {
* @see
org.apache.poi.ss.formula.DataValidationEvaluator.ValidationEnum#isValidValue(org.apache.poi.ss.usermodel.Cell,
org.apache.poi.ss.usermodel.DataValidationConstraint,
org.apache.poi.ss.formula.WorkbookEvaluator)
*/
public boolean isValidValue(Cell cell, DataValidationContext
context) {
+ // unwrapped single value
ValueEval comp =
context.getEvaluator().getWorkbookEvaluator().evaluate(context.getFormula1(),
context.getTarget(), context.getRegion());
if (comp instanceof RefEval) {
comp = ((RefEval) comp).getInnerValueEval(((RefEval)
comp).getFirstSheetIndex());
@@ -419,6 +421,7 @@ public class DataValidationEvaluator {
} catch (NumberFormatException e) {
// must be an expression, then. Overloading by Excel in the
file formats.
}
+ // note the call to the "unwrapped" version, which returns a
single value
ValueEval eval =
context.getEvaluator().getWorkbookEvaluator().evaluate(formula,
context.getTarget(), context.getRegion());
if (eval instanceof RefEval) {
eval = ((RefEval) eval).getInnerValueEval(((RefEval)
eval).getFirstSheetIndex());
Modified: poi/trunk/src/java/org/apache/poi/ss/formula/FormulaType.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/FormulaType.java?rev=1803121&r1=1803120&r2=1803121&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/FormulaType.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/FormulaType.java Wed Jul 26
22:19:58 2017
@@ -27,7 +27,7 @@ import org.apache.poi.util.Internal;
@Internal
public enum FormulaType {
/** Regular cell formula */
- CELL(0),
+ CELL(true),
/**
* A Shared Formula ("{=SUM(A1:E1*{1,2,3,4,5}}")
@@ -35,46 +35,52 @@ public enum FormulaType {
* Similar to an array formula, but stored in a SHAREDFMLA instead of
ARRAY record as a file size optimization.
* See Section 4.8 of https://www.openoffice.org/sc/excelfileformat.pdf
*/
- SHARED(1),
+ SHARED(true),
/**
* An Array formula ("{=SUM(A1:E1*{1,2,3,4,5}}")
*
https://support.office.com/en-us/article/Guidelines-and-examples-of-array-formulas-7D94A64E-3FF3-4686-9372-ECFD5CAA57C7
*/
- ARRAY(2),
+ ARRAY(false),
/** Conditional formatting */
- CONDFORMAT(3),
+ CONDFORMAT(true),
/** Named range */
- NAMEDRANGE(4),
+ NAMEDRANGE(false),
/**
* This constant is currently very specific. The exact differences from
general data
* validation formulas or conditional format formulas is not known yet
*/
- DATAVALIDATION_LIST(5);
+ DATAVALIDATION_LIST(false);
- /** @deprecated POI 3.15 beta 3. */
- private final int code;
+ /** formula is expected to return a single value vs. multiple values */
+ private final boolean isSingleValue ;
/**
* @since POI 3.15 beta 3.
- * @deprecated POI 3.15 beta 3.
- * Formula type code doesn't mean anything. Only in this class for
transitioning from a class with int constants to a true enum.
- * Remove hard-coded numbers from the enums above. */
- private FormulaType(int code) {
- this.code = code;
+ */
+ private FormulaType(boolean singleValue) {
+ this.isSingleValue = singleValue;
+ }
+
+ /**
+ * @return true if this formula type only returns single values, false if
it can return multiple values (arrays, ranges, etc.)
+ */
+ public boolean isSingleValue() {
+ return isSingleValue;
}
/**
+ * Used to transition from <code>int</code>s (possibly stored in the Excel
file) to <code>FormulaType</code>s.
+ * @param code
+ * @return FormulaType
+ * @throws IllegalArgumentException if code is out of range
* @since POI 3.15 beta 3.
- * @deprecated POI 3.15 beta 3. Used to transition code from
<code>int</code>s to <code>FormulaType</code>s.
*/
public static FormulaType forInt(int code) {
- for (FormulaType type : values()) {
- if (type.code == code) {
- return type;
- }
+ if (code >= 0 && code < values().length) {
+ return values()[code];
}
throw new IllegalArgumentException("Invalid FormulaType code: " +
code);
}
Modified:
poi/trunk/src/java/org/apache/poi/ss/formula/OperationEvaluationContext.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/OperationEvaluationContext.java?rev=1803121&r1=1803120&r2=1803121&view=diff
==============================================================================
---
poi/trunk/src/java/org/apache/poi/ss/formula/OperationEvaluationContext.java
(original)
+++
poi/trunk/src/java/org/apache/poi/ss/formula/OperationEvaluationContext.java
Wed Jul 26 22:19:58 2017
@@ -53,15 +53,22 @@ public final class OperationEvaluationCo
private final int _columnIndex;
private final EvaluationTracker _tracker;
private final WorkbookEvaluator _bookEvaluator;
-
+ private final boolean _isSingleValue;
+
public OperationEvaluationContext(WorkbookEvaluator bookEvaluator,
EvaluationWorkbook workbook, int sheetIndex, int srcRowNum,
int srcColNum, EvaluationTracker tracker) {
+ this(bookEvaluator, workbook, sheetIndex, srcRowNum, srcColNum,
tracker, true);
+ }
+
+ public OperationEvaluationContext(WorkbookEvaluator bookEvaluator,
EvaluationWorkbook workbook, int sheetIndex, int srcRowNum,
+ int srcColNum, EvaluationTracker tracker, boolean isSingleValue) {
_bookEvaluator = bookEvaluator;
_workbook = workbook;
_sheetIndex = sheetIndex;
_rowIndex = srcRowNum;
_columnIndex = srcColNum;
_tracker = tracker;
+ _isSingleValue = isSingleValue;
}
public EvaluationWorkbook getWorkbook() {
@@ -411,6 +418,14 @@ public final class OperationEvaluationCo
return _sheetIndex;
}
+ /**
+ * default true
+ * @return flag indicating whether evaluation should "unwrap" the result
to a single value based on the context row/column
+ */
+ public boolean isSingleValue() {
+ return _isSingleValue;
+ }
+
private ValueEval getExternalNameXEval(ExternalName externName, String
workbookName) {
try {
// Fetch the workbook this refers to, and the name as defined with
that
Modified: poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java?rev=1803121&r1=1803120&r2=1803121&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
(original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java Wed Jul
26 22:19:58 2017
@@ -44,11 +44,11 @@ import org.apache.poi.util.Internal;
import org.apache.poi.util.POILogFactory;
import org.apache.poi.util.POILogger;
/**
- * Evaluates formula cells.<p>
+ * Evaluates formula cells.<p/>
*
* For performance reasons, this class keeps a cache of all previously
calculated intermediate
* cell values. Be sure to call {@link #clearAllCachedResultValues()} if any
workbook cells are changed between
- * calls to evaluate~ methods on this class.<br>
+ * calls to evaluate~ methods on this class.<br/>
*
* For POI internal use only
*
@@ -529,7 +529,15 @@ public final class WorkbookEvaluator {
if (!stack.isEmpty()) {
throw new IllegalStateException("evaluation stack not empty");
}
- ValueEval result = dereferenceResult(value, ec.getRowIndex(),
ec.getColumnIndex());
+
+ // "unwrap" result to just the value relevant for the source cell if
needed
+ ValueEval result;
+ if (ec.isSingleValue()) {
+ result = dereferenceResult(value, ec.getRowIndex(),
ec.getColumnIndex());
+ } else {
+ result = value;
+ }
+
if (dbgEvaluationOutputIndent > 0) {
EVAL_LOG.log(POILogger.INFO, dbgIndentStr + "finshed eval of "
+ new CellReference(ec.getRowIndex(),
ec.getColumnIndex()).formatAsString()
@@ -595,7 +603,7 @@ public final class WorkbookEvaluator {
/**
* returns an appropriate Eval impl instance for the Ptg. The Ptg must be
* one of: Area3DPtg, AreaPtg, ReferencePtg, Ref3DPtg, IntPtg, NumberPtg,
- * StringPtg, BoolPtg <br>special Note: OperationPtg subtypes cannot be
+ * StringPtg, BoolPtg <br/>special Note: OperationPtg subtypes cannot be
* passed here!
*/
private ValueEval getEvalForPtg(Ptg ptg, OperationEvaluationContext ec) {
@@ -721,16 +729,28 @@ public final class WorkbookEvaluator {
* Evaluate a formula outside a cell value, e.g. conditional format rules
or data validation expressions
*
* @param formula to evaluate
- * @param ref defines the sheet and optionally row/column base for the
formula, if it is relative
+ * @param ref defines the optional sheet and row/column base for the
formula, if it is relative
* @return value
- * @throws IllegalArgumentException if ref does not define a sheet name to
evaluate the formula on.
*/
public ValueEval evaluate(String formula, CellReference ref) {
final String sheetName = ref == null ? null : ref.getSheetName();
- if (sheetName == null) throw new IllegalArgumentException("Sheet name
is required");
- final int sheetIndex = getWorkbook().getSheetIndex(sheetName);
- final OperationEvaluationContext ec = new
OperationEvaluationContext(this, getWorkbook(), sheetIndex, ref.getRow(),
ref.getCol(), new EvaluationTracker(_cache));
- Ptg[] ptgs = FormulaParser.parse(formula, (FormulaParsingWorkbook)
getWorkbook(), FormulaType.CELL, sheetIndex, ref.getRow());
+ int sheetIndex;
+ if (sheetName == null) {
+ sheetIndex = -1; // workbook scope only
+ } else {
+ sheetIndex = getWorkbook().getSheetIndex(sheetName);
+ }
+ int rowIndex = ref == null ? -1 : ref.getRow();
+ short colIndex = ref == null ? -1 : ref.getCol();
+ final OperationEvaluationContext ec = new OperationEvaluationContext(
+ this,
+ getWorkbook(),
+ sheetIndex,
+ rowIndex,
+ colIndex,
+ new EvaluationTracker(_cache)
+ );
+ Ptg[] ptgs = FormulaParser.parse(formula, (FormulaParsingWorkbook)
getWorkbook(), FormulaType.CELL, sheetIndex, rowIndex);
return evaluateNameFormula(ptgs, ec);
}
@@ -739,6 +759,8 @@ public final class WorkbookEvaluator {
* such as some data validation and conditional format expressions, when
those constraints apply
* to contiguous cells. When a relative formula is used, it must be
evaluated by shifting by the target
* offset position relative to the top left of the range.
+ * <p>
+ * Returns a single value e.g. a cell formula result or boolean value for
conditional formatting.
*
* @param formula
* @param target cell context for the operation
@@ -747,15 +769,37 @@ public final class WorkbookEvaluator {
* @throws IllegalArgumentException if target does not define a sheet name
to evaluate the formula on.
*/
public ValueEval evaluate(String formula, CellReference target,
CellRangeAddressBase region) {
+ return evaluate(formula, target, region, FormulaType.CELL);
+ }
+
+ /**
+ * Some expressions need to be evaluated in terms of an offset from the
top left corner of a region,
+ * such as some data validation and conditional format expressions, when
those constraints apply
+ * to contiguous cells. When a relative formula is used, it must be
evaluated by shifting by the target
+ * offset position relative to the top left of the range.
+ * <p>
+ * Returns a ValueEval that may be one or more values, such as the allowed
values for a data validation constraint.
+ *
+ * @param formula
+ * @param target cell context for the operation
+ * @param region containing the cell
+ * @return ValueEval for one or more values
+ * @throws IllegalArgumentException if target does not define a sheet name
to evaluate the formula on.
+ */
+ public ValueEval evaluateList(String formula, CellReference target,
CellRangeAddressBase region) {
+ return evaluate(formula, target, region,
FormulaType.DATAVALIDATION_LIST);
+ }
+
+ private ValueEval evaluate(String formula, CellReference target,
CellRangeAddressBase region, FormulaType formulaType) {
final String sheetName = target == null ? null : target.getSheetName();
if (sheetName == null) throw new IllegalArgumentException("Sheet name
is required");
final int sheetIndex = getWorkbook().getSheetIndex(sheetName);
- Ptg[] ptgs = FormulaParser.parse(formula, (FormulaParsingWorkbook)
getWorkbook(), FormulaType.CELL, sheetIndex, target.getRow());
+ Ptg[] ptgs = FormulaParser.parse(formula, (FormulaParsingWorkbook)
getWorkbook(), formulaType, sheetIndex, target.getRow());
adjustRegionRelativeReference(ptgs, target, region);
- final OperationEvaluationContext ec = new
OperationEvaluationContext(this, getWorkbook(), sheetIndex, target.getRow(),
target.getCol(), new EvaluationTracker(_cache));
+ final OperationEvaluationContext ec = new
OperationEvaluationContext(this, getWorkbook(), sheetIndex, target.getRow(),
target.getCol(), new EvaluationTracker(_cache), formulaType.isSingleValue());
return evaluateNameFormula(ptgs, ec);
}
Modified:
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataValidation.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataValidation.java?rev=1803121&r1=1803120&r2=1803121&view=diff
==============================================================================
---
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataValidation.java
(original)
+++
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataValidation.java
Wed Jul 26 22:19:58 2017
@@ -22,6 +22,9 @@ import java.io.IOException;
import java.math.BigDecimal;
import java.util.List;
+import org.apache.poi.ss.formula.DataValidationEvaluator;
+import org.apache.poi.ss.formula.WorkbookEvaluator;
+import org.apache.poi.ss.formula.eval.ValueEval;
import org.apache.poi.ss.usermodel.BaseTestDataValidation;
import org.apache.poi.ss.usermodel.Cell;
import org.apache.poi.ss.usermodel.CellType;
@@ -341,4 +344,13 @@ public class TestXSSFDataValidation exte
final XSSFDataValidation validation = (XSSFDataValidation)
dataValidationHelper.createValidation(constraint, new CellRangeAddressList(0,
0, 0, 0));
return validation;
}
+
+ @Test
+ public void testTableBasedValidationList() {
+ XSSFWorkbook wb =
XSSFTestDataSamples.openSampleWorkbook("dataValidationTableRange.xlsx");
+ XSSFFormulaEvaluator fEval =
wb.getCreationHelper().createFormulaEvaluator();
+ DataValidationEvaluator dve = new DataValidationEvaluator(wb, fEval);
+ List<ValueEval> values = dve.getValidationValuesForCell(new
CellReference("County Ranking", 8, 6, false, false));
+ assertEquals("wrong # of valid values", 32, values.size());
+ }
}
Added: poi/trunk/test-data/spreadsheet/dataValidationTableRange.xlsx
URL:
http://svn.apache.org/viewvc/poi/trunk/test-data/spreadsheet/dataValidationTableRange.xlsx?rev=1803121&view=auto
==============================================================================
Binary file - no diff available.
Propchange: poi/trunk/test-data/spreadsheet/dataValidationTableRange.xlsx
------------------------------------------------------------------------------
svn:mime-type = application/octet-stream
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
