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 2019/03/16 00:27:13 UTC
svn commit: r1855619 - in /poi/trunk/src:
java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java
java/org/apache/poi/ss/formula/WorkbookEvaluator.java
ooxml/testcases/org/apache/poi/ss/usermodel/ConditionalFormattingEvalTest.java
Author: gwoolsey
Date: Sat Mar 16 00:27:13 2019
New Revision: 1855619
URL: http://svn.apache.org/viewvc?rev=1855619&view=rev
Log:
#63264 Conditional Format rule evaluation calculates relative references incorrectly
Fixing the offset calculation and passing the top-left-most region rather than the region matching the current cell fixed these cases. I've augmented unit tests to check for them to avoid future regressions.
Modified:
poi/trunk/src/java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java
poi/trunk/src/java/org/apache/poi/ss/formula/WorkbookEvaluator.java
poi/trunk/src/ooxml/testcases/org/apache/poi/ss/usermodel/ConditionalFormattingEvalTest.java
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=1855619&r1=1855618&r2=1855619&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 Sat Mar 16 00:27:13 2019
@@ -69,6 +69,9 @@ public class EvaluationConditionalFormat
/* cached values */
private final CellRangeAddress[] regions;
+
+ private CellRangeAddress topLeftRegion;
+
/**
* Depending on the rule type, it may want to know about certain values in the region when evaluating {@link #matches(CellReference)},
* such as top 10, unique, duplicate, average, etc. This collection stores those if needed so they are not repeatedly calculated
@@ -114,6 +117,14 @@ public class EvaluationConditionalFormat
this.priority = rule.getPriority();
this.regions = regions;
+
+ for (CellRangeAddress region : regions) {
+ if (topLeftRegion == null) topLeftRegion = region;
+ else if (region.getFirstColumn() < topLeftRegion.getFirstColumn()
+ || region.getFirstRow() < topLeftRegion.getFirstRow()) {
+ topLeftRegion = region;
+ }
+ }
formula1 = rule.getFormula1();
formula2 = rule.getFormula2();
@@ -316,13 +327,13 @@ public class EvaluationConditionalFormat
if (ruleType.equals(ConditionType.CELL_VALUE_IS)) {
// undefined cells never match a VALUE_IS condition
if (cell == null) return false;
- return checkValue(cell, region);
+ return checkValue(cell, topLeftRegion);
}
if (ruleType.equals(ConditionType.FORMULA)) {
- return checkFormula(ref, region);
+ return checkFormula(ref, topLeftRegion);
}
if (ruleType.equals(ConditionType.FILTER)) {
- return checkFilter(cell, ref, region);
+ return checkFilter(cell, ref, topLeftRegion);
}
// TODO: anything else, we don't handle yet, such as top 10
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=1855619&r1=1855618&r2=1855619&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 Sat Mar 16 00:27:13 2019
@@ -880,20 +880,16 @@ public final class WorkbookEvaluator {
* <p><pre>formula ref + current cell ref</pre></p>
* @param ptgs
* @param target cell within the region to use.
- * @param region containing the cell
+ * @param region containing the cell, OR, for conditional format rules with multiple ranges, the region with the top-left-most cell
* @return true if any Ptg references were shifted
* @throws IndexOutOfBoundsException if the resulting shifted row/column indexes are over the document format limits
* @throws IllegalArgumentException if target is not within region.
*/
protected boolean adjustRegionRelativeReference(Ptg[] ptgs, CellReference target, CellRangeAddressBase region) {
- if (! region.isInRange(target)) {
- throw new IllegalArgumentException(target + " is not within " + region);
- }
+ // region may not be the one that contains the target, if a conditional formatting rule applies to multiple regions
- //return adjustRegionRelativeReference(ptgs, target.getRow() - region.getFirstRow(), target.getCol() - region.getFirstColumn());
-
- int deltaRow = target.getRow();
- int deltaColumn = target.getCol();
+ int deltaRow = target.getRow() - region.getFirstRow();
+ int deltaColumn = target.getCol() - region.getFirstColumn();
boolean shifted = false;
for (Ptg ptg : ptgs) {
@@ -902,7 +898,7 @@ public final class WorkbookEvaluator {
RefPtgBase ref = (RefPtgBase) ptg;
// re-calculate cell references
final SpreadsheetVersion version = _workbook.getSpreadsheetVersion();
- if (ref.isRowRelative()) {
+ if (ref.isRowRelative() && deltaRow > 0) {
final int rowIndex = ref.getRow() + deltaRow;
if (rowIndex > version.getMaxRows()) {
throw new IndexOutOfBoundsException(version.name() + " files can only have " + version.getMaxRows() + " rows, but row " + rowIndex + " was requested.");
@@ -910,7 +906,7 @@ public final class WorkbookEvaluator {
ref.setRow(rowIndex);
shifted = true;
}
- if (ref.isColRelative()) {
+ if (ref.isColRelative() && deltaColumn > 0) {
final int colIndex = ref.getColumn() + deltaColumn;
if (colIndex > version.getMaxColumns()) {
throw new IndexOutOfBoundsException(version.name() + " files can only have " + version.getMaxColumns() + " columns, but column " + colIndex + " was requested.");
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=1855619&r1=1855618&r2=1855619&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 Sat Mar 16 00:27:13 2019
@@ -19,6 +19,7 @@ package org.apache.poi.ss.usermodel;
import java.io.IOException;
+import java.util.Date;
import java.util.List;
import org.apache.poi.ss.formula.ConditionalFormattingEvaluator;
@@ -117,6 +118,28 @@ public class ConditionalFormattingEvalTe
getRulesFor(8,2);
assertEquals("wrong # of rules for " + ref, 1, rules.size());
+ sheet = wb.getSheet("Compare to totals");
+ getRulesFor(3, 2);
+ assertEquals("wrong # of rules for " + ref, 1, rules.size());
+ assertEquals("wrong fg color for " + ref, "FFFF0000", getColor(rules.get(0).getRule().getFontFormatting().getFontColor()));
+ getRulesFor(3, 3);
+ assertEquals("wrong # of rules for " + ref, 0, rules.size());
+ getRulesFor(15, 4);
+ assertEquals("wrong # of rules for " + ref, 0, rules.size());
+ getRulesFor(16, 1);
+ assertEquals("wrong # of rules for " + ref, 1, rules.size());
+ assertEquals("wrong fg color for " + ref, "FFFF0000", getColor(rules.get(0).getRule().getFontFormatting().getFontColor()));
+
+ sheet = wb.getSheet("Products3");
+ sheet.getRow(8).getCell(0).setCellValue(new Date());
+ getRulesFor(8, 0);
+ assertEquals("wrong # of rules for " + ref, 1, rules.size());
+ getRulesFor(8, 3);
+ assertEquals("wrong # of rules for " + ref, 1, rules.size());
+
+ sheet = wb.getSheet("Customers2");
+ getRulesFor(3, 0);
+ assertEquals("wrong # of rules for " + ref, 0, rules.size());
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org