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 2017/08/16 23:52:28 UTC
svn commit: r1805245 - in /poi/trunk: src/java/org/apache/poi/ss/formula/
src/ooxml/testcases/org/apache/poi/ss/usermodel/ test-data/spreadsheet/
Author: gwoolsey
Date: Wed Aug 16 23:52:27 2017
New Revision: 1805245
URL: http://svn.apache.org/viewvc?rev=1805245&view=rev
Log:
Bug 61431 - Conditional formatting evaluation ignores undefined cells
now evaluating based on cell references instead, and watching out for undefined cells in rules that require a cell.
Added unit test based on previously failing file.
Added:
poi/trunk/test-data/spreadsheet/conditional_formatting_with_formula_on_second_sheet.xlsx (with props)
Modified:
poi/trunk/src/java/org/apache/poi/ss/formula/ConditionalFormattingEvaluator.java
poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java
poi/trunk/src/ooxml/testcases/org/apache/poi/ss/usermodel/ConditionalFormattingEvalTest.java
Modified: poi/trunk/src/java/org/apache/poi/ss/formula/ConditionalFormattingEvaluator.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/ConditionalFormattingEvaluator.java?rev=1805245&r1=1805244&r2=1805245&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/ConditionalFormattingEvaluator.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/ConditionalFormattingEvaluator.java Wed Aug 16 23:52:27 2017
@@ -150,57 +150,16 @@ public class ConditionalFormattingEvalua
* or null if none apply
*/
public List<EvaluationConditionalFormatRule> getConditionalFormattingForCell(final CellReference cellRef) {
- String sheetName = cellRef.getSheetName();
- Sheet sheet = null;
- if (sheetName == null) {
- sheet = workbook.getSheetAt(workbook.getActiveSheetIndex());
- } else {
- sheet = workbook.getSheet(sheetName);
- }
-
- final Cell cell = SheetUtil.getCell(sheet, cellRef.getRow(), cellRef.getCol());
-
- if (cell == null) {
- return Collections.emptyList();
- }
-
- return getConditionalFormattingForCell(cell, cellRef);
- }
-
- /**
- * This checks all applicable {@link ConditionalFormattingRule}s for the cell's sheet,
- * in defined "priority" order, returning the matches if any. This is a property currently
- * not exposed from <code>CTCfRule</code> in <code>XSSFConditionalFormattingRule</code>.
- * <p>
- * Most cells will have zero or one applied rule, but it is possible to define multiple rules
- * that apply at the same time to the same cell, thus the List result.
- * <p>
- * Note that to properly apply conditional rules, care must be taken to offset the base
- * formula by the relative position of the current cell, or the wrong value is checked.
- * This is handled by {@link WorkbookEvaluator#evaluate(String, CellReference, CellRangeAddressBase)}.
- *
- * @param cell
- * @return Unmodifiable List of {@link EvaluationConditionalFormatRule}s that apply to the current cell value,
- * in priority order, as evaluated by Excel (smallest priority # for XSSF, definition order for HSSF),
- * or null if none apply
- */
- public List<EvaluationConditionalFormatRule> getConditionalFormattingForCell(Cell cell) {
- return getConditionalFormattingForCell(cell, getRef(cell));
- }
-
- /**
- * We need both, and can derive one from the other, but this is to avoid duplicate work
- *
- * @param cell
- * @param ref
- * @return unmodifiable list of matching rules
- */
- private List<EvaluationConditionalFormatRule> getConditionalFormattingForCell(Cell cell, CellReference ref) {
- List<EvaluationConditionalFormatRule> rules = values.get(ref);
+ List<EvaluationConditionalFormatRule> rules = values.get(cellRef);
if (rules == null) {
// compute and cache them
rules = new ArrayList<EvaluationConditionalFormatRule>();
+
+ Sheet sheet = null;
+ if (cellRef.getSheetName() != null) sheet = workbook.getSheet(cellRef.getSheetName());
+ else sheet = workbook.getSheetAt(workbook.getActiveSheetIndex());
+
/*
* Per Excel help:
* https://support.office.com/en-us/article/Manage-conditional-formatting-rule-precedence-e09711a3-48df-4bcb-b82c-9d8b8b22463d#__toc269129417
@@ -208,24 +167,45 @@ public class ConditionalFormattingEvalua
* thus the explicit ordering of the rule lists in #getFormattingRulesForSheet(Sheet)
*/
boolean stopIfTrue = false;
- for (EvaluationConditionalFormatRule rule : getRules(cell.getSheet())) {
+ for (EvaluationConditionalFormatRule rule : getRules(sheet)) {
if (stopIfTrue) {
continue; // a previous rule matched and wants no more evaluations
}
- if (rule.matches(cell)) {
+ if (rule.matches(cellRef)) {
rules.add(rule);
stopIfTrue = rule.getRule().getStopIfTrue();
}
}
Collections.sort(rules);
- values.put(ref, rules);
+ values.put(cellRef, rules);
}
return Collections.unmodifiableList(rules);
}
+ /**
+ * This checks all applicable {@link ConditionalFormattingRule}s for the cell's sheet,
+ * in defined "priority" order, returning the matches if any. This is a property currently
+ * not exposed from <code>CTCfRule</code> in <code>XSSFConditionalFormattingRule</code>.
+ * <p>
+ * Most cells will have zero or one applied rule, but it is possible to define multiple rules
+ * that apply at the same time to the same cell, thus the List result.
+ * <p>
+ * Note that to properly apply conditional rules, care must be taken to offset the base
+ * formula by the relative position of the current cell, or the wrong value is checked.
+ * This is handled by {@link WorkbookEvaluator#evaluate(String, CellReference, CellRangeAddressBase)}.
+ *
+ * @param cell
+ * @return Unmodifiable List of {@link EvaluationConditionalFormatRule}s that apply to the current cell value,
+ * in priority order, as evaluated by Excel (smallest priority # for XSSF, definition order for HSSF),
+ * or null if none apply
+ */
+ public List<EvaluationConditionalFormatRule> getConditionalFormattingForCell(Cell cell) {
+ return getConditionalFormattingForCell(getRef(cell));
+ }
+
public static CellReference getRef(Cell cell) {
return new CellReference(cell.getSheet().getSheetName(), cell.getRowIndex(), cell.getColumnIndex(), false, false);
}
Modified: poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java?rev=1805245&r1=1805244&r2=1805245&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java Wed Aug 16 23:52:27 2017
@@ -45,6 +45,7 @@ import org.apache.poi.ss.usermodel.Excel
import org.apache.poi.ss.usermodel.Row;
import org.apache.poi.ss.usermodel.Sheet;
import org.apache.poi.ss.util.CellRangeAddress;
+import org.apache.poi.ss.util.CellReference;
/**
* Abstracted and cached version of a Conditional Format rule for use with a
@@ -262,14 +263,14 @@ public class EvaluationConditionalFormat
}
/**
- * @param cell
+ * @param ref
* @return true if this rule evaluates to true for the given cell
*/
- /* package */ boolean matches(Cell cell) {
+ /* package */ boolean matches(CellReference ref) {
// first check that it is in one of the regions defined for this format
CellRangeAddress region = null;
for (CellRangeAddress r : regions) {
- if (r.isInRange(cell)) {
+ if (r.isInRange(ref)) {
region = r;
break;
}
@@ -290,14 +291,22 @@ public class EvaluationConditionalFormat
return true;
}
+ Cell cell = null;
+ final Row row = sheet.getRow(ref.getRow());
+ if (row != null) {
+ cell = row.getCell(ref.getCol());
+ }
+
if (ruleType.equals(ConditionType.CELL_VALUE_IS)) {
+ // undefined cells never match a VALUE_IS condition
+ if (cell == null) return false;
return checkValue(cell, region);
}
if (ruleType.equals(ConditionType.FORMULA)) {
- return checkFormula(cell, region);
+ return checkFormula(ref, region);
}
if (ruleType.equals(ConditionType.FILTER)) {
- return checkFilter(cell, region);
+ return checkFilter(cell, ref, region);
}
// TODO: anything else, we don't handle yet, such as top 10
@@ -361,12 +370,12 @@ public class EvaluationConditionalFormat
return comp;
}
/**
- * @param cell needed for offsets from region anchor
+ * @param cell needed for offsets from region anchor - may be null!
* @param region for adjusting relative formulas
* @return true/false using the same rules as Data Validation evaluations
*/
- private boolean checkFormula(Cell cell, CellRangeAddress region) {
- ValueEval comp = unwrapEval(workbookEvaluator.evaluate(rule.getFormula1(), ConditionalFormattingEvaluator.getRef(cell), region));
+ private boolean checkFormula(CellReference ref, CellRangeAddress region) {
+ ValueEval comp = unwrapEval(workbookEvaluator.evaluate(rule.getFormula1(), ref, region));
// Copied for now from DataValidationEvaluator.ValidationEnum.FORMULA#isValidValue()
if (comp instanceof BlankEval) {
@@ -386,11 +395,13 @@ public class EvaluationConditionalFormat
return false; // anything else is false, such as text
}
- private boolean checkFilter(Cell cell, CellRangeAddress region) {
+ private boolean checkFilter(Cell cell, CellReference ref, CellRangeAddress region) {
final ConditionFilterType filterType = rule.getConditionFilterType();
if (filterType == null) {
return false;
}
+
+ final ValueAndFormat cv = getCellValue(cell);
// TODO: this could/should be delegated to the Enum type, but that's in the usermodel package,
// we may not want evaluation code there. Of course, maybe the enum should go here in formula,
@@ -403,8 +414,7 @@ public class EvaluationConditionalFormat
// from testing, Excel only operates on numbers and dates (which are stored as numbers) in the range.
// numbers stored as text are ignored, but numbers formatted as text are treated as numbers.
- final ValueAndFormat cv10 = getCellValue(cell);
- if (! cv10.isNumber()) {
+ if (! cv.isNumber()) {
return false;
}
@@ -430,7 +440,7 @@ public class EvaluationConditionalFormat
return new HashSet<ValueAndFormat>(allValues.subList(0, limit));
}
- }).contains(cv10);
+ }).contains(cv);
case UNIQUE_VALUES:
// Per Excel help, "duplicate" means matching value AND format
// https://support.office.com/en-us/article/Filter-for-unique-values-or-remove-duplicate-values-ccf664b0-81d6-449b-bbe1-8daaec1e83c2
@@ -455,7 +465,7 @@ public class EvaluationConditionalFormat
return unique;
}
- }).contains(getCellValue(cell));
+ }).contains(cv);
case DUPLICATE_VALUES:
// Per Excel help, "duplicate" means matching value AND format
// https://support.office.com/en-us/article/Filter-for-unique-values-or-remove-duplicate-values-ccf664b0-81d6-449b-bbe1-8daaec1e83c2
@@ -478,7 +488,7 @@ public class EvaluationConditionalFormat
}
return dup;
}
- }).contains(getCellValue(cell));
+ }).contains(cv);
case ABOVE_AVERAGE:
// from testing, Excel only operates on numbers and dates (which are stored as numbers) in the range.
// numbers stored as text are ignored, but numbers formatted as text are treated as numbers.
@@ -507,7 +517,6 @@ public class EvaluationConditionalFormat
}
}));
- final ValueAndFormat cv = getCellValue(cell);
Double val = cv.isNumber() ? cv.getValue() : null;
if (val == null) {
return false;
@@ -541,19 +550,19 @@ public class EvaluationConditionalFormat
return op != null && op.isValid(val, comp, null);
case CONTAINS_TEXT:
// implemented both by a cfRule "text" attribute and a formula. Use the formula.
- return checkFormula(cell, region);
+ return checkFormula(ref, region);
case NOT_CONTAINS_TEXT:
// implemented both by a cfRule "text" attribute and a formula. Use the formula.
- return checkFormula(cell, region);
+ return checkFormula(ref, region);
case BEGINS_WITH:
// implemented both by a cfRule "text" attribute and a formula. Use the formula.
- return checkFormula(cell, region);
+ return checkFormula(ref, region);
case ENDS_WITH:
// implemented both by a cfRule "text" attribute and a formula. Use the formula.
- return checkFormula(cell, region);
+ return checkFormula(ref, region);
case CONTAINS_BLANKS:
try {
- String v = cell.getStringCellValue();
+ String v = cv.getString();
// see TextFunction.TRIM for implementation
return v == null || v.trim().length() == 0;
} catch (Exception e) {
@@ -562,7 +571,7 @@ public class EvaluationConditionalFormat
}
case NOT_CONTAINS_BLANKS:
try {
- String v = cell.getStringCellValue();
+ String v = cv.getString();
// see TextFunction.TRIM for implementation
return v != null && v.trim().length() > 0;
} catch (Exception e) {
@@ -575,7 +584,7 @@ public class EvaluationConditionalFormat
return cell == null || ! DataValidationEvaluator.isType(cell, CellType.ERROR);
case TIME_PERIOD:
// implemented both by a cfRule "text" attribute and a formula. Use the formula.
- return checkFormula(cell, region);
+ return checkFormula(ref, region);
default:
return false;
}
@@ -627,7 +636,7 @@ public class EvaluationConditionalFormat
return new ValueAndFormat(cell.getStringCellValue(), cell.getCellStyle().getDataFormatString());
}
}
- return null;
+ return new ValueAndFormat("", "");
}
/**
* instances evaluate the values for a region and return the positive matches for the function type.
@@ -754,6 +763,10 @@ public class EvaluationConditionalFormat
return value;
}
+ public String getString() {
+ return string;
+ }
+
@Override
public boolean equals(Object obj) {
if (!(obj instanceof ValueAndFormat)) {
Modified: poi/trunk/src/ooxml/testcases/org/apache/poi/ss/usermodel/ConditionalFormattingEvalTest.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/ss/usermodel/ConditionalFormattingEvalTest.java?rev=1805245&r1=1805244&r2=1805245&view=diff
==============================================================================
--- poi/trunk/src/ooxml/testcases/org/apache/poi/ss/usermodel/ConditionalFormattingEvalTest.java (original)
+++ poi/trunk/src/ooxml/testcases/org/apache/poi/ss/usermodel/ConditionalFormattingEvalTest.java Wed Aug 16 23:52:27 2017
@@ -117,6 +117,18 @@ public class ConditionalFormattingEvalTe
assertEquals("wrong # of rules for " + ref, 1, rules.size());
}
+
+ @Test
+ public void testFormattingOnUndefinedCell() throws Exception {
+ wb = XSSFTestDataSamples.openSampleWorkbook("conditional_formatting_with_formula_on_second_sheet.xlsx");
+ formulaEval = new XSSFFormulaEvaluator(wb);
+ cfe = new ConditionalFormattingEvaluator(wb, formulaEval);
+
+ sheet = wb.getSheet("Sales Plan");
+ getRulesFor(9,2);
+ assertNotEquals("No rules for " + ref, 0, rules.size());
+ assertEquals("wrong bg color for " + ref, "FFFFFF00", getColor(rules.get(0).getRule().getPatternFormatting().getFillBackgroundColorColor()));
+ }
private List<EvaluationConditionalFormatRule> getRulesFor(int row, int col) {
ref = new CellReference(sheet.getSheetName(), row, col, false, false);
Added: poi/trunk/test-data/spreadsheet/conditional_formatting_with_formula_on_second_sheet.xlsx
URL: http://svn.apache.org/viewvc/poi/trunk/test-data/spreadsheet/conditional_formatting_with_formula_on_second_sheet.xlsx?rev=1805245&view=auto
==============================================================================
Binary file - no diff available.
Propchange: poi/trunk/test-data/spreadsheet/conditional_formatting_with_formula_on_second_sheet.xlsx
------------------------------------------------------------------------------
svn:mime-type = application/octet-stream
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org