diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java b/src/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java index ebbec778c..27e152b3a 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java @@ -305,6 +305,10 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin return null; } + public String getText() { + return null; // not available here, unless it exists and is unimplemented in cfRuleRecord + } + protected String toFormulaString(Ptg[] parsedExpression) { return toFormulaString(parsedExpression, workbook); } diff --git a/src/java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java b/src/java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java index d90e00b3d..805763e38 100644 --- a/src/java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java +++ b/src/java/org/apache/poi/ss/formula/EvaluationConditionalFormatRule.java @@ -17,12 +17,15 @@ package org.apache.poi.ss.formula; +import java.text.DecimalFormat; +import java.text.DecimalFormatSymbols; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; @@ -82,10 +85,13 @@ public class EvaluationConditionalFormatRule implements Comparable 0) { eval2 = unwrapEval(workbookEvaluator.evaluate(f2, ConditionalFormattingEvaluator.getRef(cell), region)); } // we assume the cell has been evaluated, and the current formula value stored - if (DataValidationEvaluator.isType(cell, CellType.BOOLEAN)) { - if (eval instanceof BoolEval && (eval2 == null || eval2 instanceof BoolEval) ) { - return operator.isValid(cell.getBooleanCellValue(), ((BoolEval) eval).getBooleanValue(), eval2 == null ? null : ((BoolEval) eval2).getBooleanValue()); - } - return false; // wrong types + if (DataValidationEvaluator.isType(cell, CellType.BOOLEAN) + && (eval == BlankEval.instance || eval instanceof BoolEval) + && (eval2 == BlankEval.instance || eval2 instanceof BoolEval) + ) { + return operator.isValid(cell.getBooleanCellValue(), eval == BlankEval.instance ? null : ((BoolEval) eval).getBooleanValue(), eval2 == BlankEval.instance ? null : ((BoolEval) eval2).getBooleanValue()); } - if (DataValidationEvaluator.isType(cell, CellType.NUMERIC)) { - if (eval instanceof NumberEval && (eval2 == null || eval2 instanceof NumberEval) ) { - return operator.isValid(cell.getNumericCellValue(), ((NumberEval) eval).getNumberValue(), eval2 == null ? null : ((NumberEval) eval2).getNumberValue()); - } - return false; // wrong types + if (DataValidationEvaluator.isType(cell, CellType.NUMERIC) + && (eval == BlankEval.instance || eval instanceof NumberEval ) + && (eval2 == BlankEval.instance || eval2 instanceof NumberEval) + ) { + return operator.isValid(cell.getNumericCellValue(), eval == BlankEval.instance ? null : ((NumberEval) eval).getNumberValue(), eval2 == BlankEval.instance ? null : ((NumberEval) eval2).getNumberValue()); } - if (DataValidationEvaluator.isType(cell, CellType.STRING)) { - if (eval instanceof StringEval && (eval2 == null || eval2 instanceof StringEval) ) { - return operator.isValid(cell.getStringCellValue(), ((StringEval) eval).getStringValue(), eval2 == null ? null : ((StringEval) eval2).getStringValue()); - } - return false; // wrong types + if (DataValidationEvaluator.isType(cell, CellType.STRING) + && (eval == BlankEval.instance || eval instanceof StringEval ) + && (eval2 == BlankEval.instance || eval2 instanceof StringEval) + ) { + return operator.isValid(cell.getStringCellValue(), eval == BlankEval.instance ? null : ((StringEval) eval).getStringValue(), eval2 == BlankEval.instance ? null : ((StringEval) eval2).getStringValue()); } - // should not get here, but in case... - return false; + return operator.isValidForIncompatibleTypes(); } private ValueEval unwrapEval(ValueEval eval) { @@ -399,7 +415,7 @@ public class EvaluationConditionalFormatRule implements Comparable avgSet = new LinkedHashSet<>(1); - avgSet.add(new ValueAndFormat(Double.valueOf(allValues.size() == 0 ? 0 : total / allValues.size()), null)); + avgSet.add(new ValueAndFormat(Double.valueOf(allValues.size() == 0 ? 0 : total / allValues.size()), null, decimalTextFormat)); final double stdDev = allValues.size() <= 1 ? 0 : ((NumberEval) AggregateFunction.STDEV.evaluate(pop, 0, 0)).getNumberValue(); - avgSet.add(new ValueAndFormat(Double.valueOf(stdDev), null)); + avgSet.add(new ValueAndFormat(Double.valueOf(stdDev), null, decimalTextFormat)); return avgSet; } })); @@ -542,17 +558,17 @@ public class EvaluationConditionalFormatRule implements Comparable> boolean isValid(C cellValue, C v1, C v2) { + if (v1 == null) { + if (cellValue instanceof Number) { + // use zero for null + double n1 = 0; + double n2 = v2 == null ? 0 : ((Number) v2).doubleValue(); + return Double.compare( ((Number) cellValue).doubleValue(), n1) >= 0 && Double.compare(((Number) cellValue).doubleValue(), n2) <= 0; + } else if (cellValue instanceof String) { + String n1 = ""; + String n2 = v2 == null ? "" : (String) v2; + return ((String) cellValue).compareToIgnoreCase(n1) >= 0 && ((String) cellValue).compareToIgnoreCase(n2) <= 0; + } else if (cellValue instanceof Boolean) return false; + return false; // just in case - not a typical possibility + } return cellValue.compareTo(v1) >= 0 && cellValue.compareTo(v2) <= 0; } }, NOT_BETWEEN { @Override public > boolean isValid(C cellValue, C v1, C v2) { + if (v1 == null) { + if (cellValue instanceof Number) { + // use zero for null + double n1 = 0; + double n2 = v2 == null ? 0 : ((Number) v2).doubleValue(); + return Double.compare( ((Number) cellValue).doubleValue(), n1) < 0 || Double.compare(((Number) cellValue).doubleValue(), n2) > 0; + } else if (cellValue instanceof String) { + String n1 = ""; + String n2 = v2 == null ? "" : (String) v2; + return ((String) cellValue).compareToIgnoreCase(n1) < 0 || ((String) cellValue).compareToIgnoreCase(n2) > 0; + } else if (cellValue instanceof Boolean) return true; + return false; // just in case - not a typical possibility + } return cellValue.compareTo(v1) < 0 || cellValue.compareTo(v2) > 0; } + + public boolean isValidForIncompatibleTypes() { + return true; + } }, EQUAL { @Override public > boolean isValid(C cellValue, C v1, C v2) { + if (v1 == null) { + if (cellValue instanceof Number) { + // use zero for null + return Double.compare( ((Number) cellValue).doubleValue(), 0) == 0; + } else if (cellValue instanceof String) { + return false; // even an empty string is not equal the empty cell, only another empty cell is, handled higher up + } else if (cellValue instanceof Boolean) return false; + return false; // just in case - not a typical possibility + } // need to avoid instanceof, to work around a 1.6 compiler bug if (cellValue.getClass() == String.class) { return cellValue.toString().compareToIgnoreCase(v1.toString()) == 0; @@ -684,34 +739,77 @@ public class EvaluationConditionalFormatRule implements Comparable> boolean isValid(C cellValue, C v1, C v2) { + if (v1 == null) { + return true; // non-null not equal null, returns true + } // need to avoid instanceof, to work around a 1.6 compiler bug if (cellValue.getClass() == String.class) { return cellValue.toString().compareToIgnoreCase(v1.toString()) == 0; } return cellValue.compareTo(v1) != 0; } + + public boolean isValidForIncompatibleTypes() { + return true; + } }, GREATER_THAN { @Override public > boolean isValid(C cellValue, C v1, C v2) { + if (v1 == null) { + if (cellValue instanceof Number) { + // use zero for null + return Double.compare( ((Number) cellValue).doubleValue(), 0) > 0; + } else if (cellValue instanceof String) { + return true; // non-null string greater than empty cell + } else if (cellValue instanceof Boolean) return true; + return false; // just in case - not a typical possibility + } return cellValue.compareTo(v1) > 0; } }, LESS_THAN { @Override public > boolean isValid(C cellValue, C v1, C v2) { + if (v1 == null) { + if (cellValue instanceof Number) { + // use zero for null + return Double.compare( ((Number) cellValue).doubleValue(), 0) < 0; + } else if (cellValue instanceof String) { + return false; // non-null string greater than empty cell + } else if (cellValue instanceof Boolean) return false; + return false; // just in case - not a typical possibility + } return cellValue.compareTo(v1) < 0; } }, GREATER_OR_EQUAL { @Override public > boolean isValid(C cellValue, C v1, C v2) { + if (v1 == null) { + if (cellValue instanceof Number) { + // use zero for null + return Double.compare( ((Number) cellValue).doubleValue(), 0) >= 0; + } else if (cellValue instanceof String) { + return true; // non-null string greater than empty cell + } else if (cellValue instanceof Boolean) return true; + return false; // just in case - not a typical possibility + } return cellValue.compareTo(v1) >= 0; } }, LESS_OR_EQUAL { @Override public > boolean isValid(C cellValue, C v1, C v2) { + if (v1 == null) { + if (cellValue instanceof Number) { + // use zero for null + return Double.compare( ((Number) cellValue).doubleValue(), 0) <= 0; + } else if (cellValue instanceof String) { + return false; // non-null string not less than empty cell + } else if (cellValue instanceof Boolean) return false; // for completeness + return false; // just in case - not a typical possibility + } return cellValue.compareTo(v1) <= 0; } }, @@ -720,11 +818,20 @@ public class EvaluationConditionalFormatRule implements Comparable> boolean isValid(C cellValue, C v1, C v2); + + /** + * Called when the cell and comparison values are of different data types + * Needed for negation operators, which should return true. + * @return true if this comparison is true when the types to compare are different + */ + public boolean isValidForIncompatibleTypes() { + return false; + } } /** @@ -735,17 +842,20 @@ public class EvaluationConditionalFormatRule implements Comparable
formula ref + range top-left + current cell range offset

+ * which simplifies to + * 

formula ref + current cell ref

* @param ptgs * @param target cell within the region to use. * @param region containing the cell @@ -854,22 +859,11 @@ public final class WorkbookEvaluator { throw new IllegalArgumentException(target + " is not within " + region); } - return adjustRegionRelativeReference(ptgs, target.getRow() - region.getFirstRow(), target.getCol() - region.getFirstColumn()); - } - - /** - * Adjust the formula relative cell references by a given delta - * @param ptgs - * @param deltaRow target row offset from the top left cell of a region - * @param deltaColumn target column offset from the top left cell of a region - * @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 either of the deltas are negative, as the assumption is we are shifting formulas - * relative to the top left cell of a region. - */ - protected boolean adjustRegionRelativeReference(Ptg[] ptgs, int deltaRow, int deltaColumn) { - if (deltaRow < 0) throw new IllegalArgumentException("offset row must be positive"); - if (deltaColumn < 0) throw new IllegalArgumentException("offset column must be positive"); + //return adjustRegionRelativeReference(ptgs, target.getRow() - region.getFirstRow(), target.getCol() - region.getFirstColumn()); + + int deltaRow = target.getRow(); + int deltaColumn = target.getCol(); + boolean shifted = false; for (Ptg ptg : ptgs) { // base class for cell reference "things" diff --git a/src/java/org/apache/poi/ss/usermodel/ConditionalFormattingRule.java b/src/java/org/apache/poi/ss/usermodel/ConditionalFormattingRule.java index b008dd566..2f19d9e3e 100644 --- a/src/java/org/apache/poi/ss/usermodel/ConditionalFormattingRule.java +++ b/src/java/org/apache/poi/ss/usermodel/ConditionalFormattingRule.java @@ -152,6 +152,13 @@ public interface ConditionalFormattingRule extends DifferentialStyleProvider { */ String getFormula2(); + /** + * XSSF rules store textual condition values as an attribute and also as a formula that needs shifting. Using the attribute is simpler/faster. + * HSSF rules don't have this and return null. We can fall back on the formula for those (AFAIK). + * @return condition text if it exists, or null + */ + String getText(); + /** * The priority of the rule, if defined, otherwise 0. *

diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFConditionalFormattingRule.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFConditionalFormattingRule.java index f3904406e..2384f85e4 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFConditionalFormattingRule.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFConditionalFormattingRule.java @@ -417,6 +417,10 @@ public class XSSFConditionalFormattingRule implements ConditionalFormattingRule return _cfRule.sizeOfFormulaArray() == 2 ? _cfRule.getFormulaArray(1) : null; } + public String getText() { + return _cfRule.getText(); + } + /** * Conditional format rules don't define stripes, so always 0 * @see org.apache.poi.ss.usermodel.DifferentialStyleProvider#getStripeSize() diff --git a/src/ooxml/testcases/org/apache/poi/ss/usermodel/ConditionalFormattingEvalTest.java b/src/ooxml/testcases/org/apache/poi/ss/usermodel/ConditionalFormattingEvalTest.java index d54ba60c8..3b1155fe7 100644 --- a/src/ooxml/testcases/org/apache/poi/ss/usermodel/ConditionalFormattingEvalTest.java +++ b/src/ooxml/testcases/org/apache/poi/ss/usermodel/ConditionalFormattingEvalTest.java @@ -23,6 +23,7 @@ import java.util.List; import org.apache.poi.ss.formula.ConditionalFormattingEvaluator; import org.apache.poi.ss.formula.EvaluationConditionalFormatRule; +import org.apache.poi.ss.formula.eval.NotImplementedException; import org.apache.poi.ss.util.CellReference; import org.apache.poi.xssf.XSSFTestDataSamples; import org.apache.poi.xssf.usermodel.XSSFColor; @@ -130,6 +131,64 @@ public class ConditionalFormattingEvalTest { assertEquals("wrong bg color for " + ref, "FFFFFF00", getColor(rules.get(0).getRule().getPatternFormatting().getFillBackgroundColorColor())); } + @Test + public void testRepeatedEval() throws Exception { + wb = XSSFTestDataSamples.openSampleWorkbook("test_conditional_formatting.xlsx"); + formulaEval = new XSSFFormulaEvaluator(wb); + cfe = new ConditionalFormattingEvaluator(wb, formulaEval); + + sheet = wb.getSheetAt(0); + try { + getRulesFor(2, 1); + fail("Got rules when an unsupported function error was expected."); + } catch (NotImplementedException e) { + // expected + } + + try { + getRulesFor(2, 1); + fail("Got rules the second time when an unsupported function error was expected."); + } catch (NotImplementedException e) { + // expected + } + + } + + @Test + public void testCellValueIsWrongType() throws Exception { + wb = XSSFTestDataSamples.openSampleWorkbook("conditional_formatting_cell_is.xlsx"); + formulaEval = new XSSFFormulaEvaluator(wb); + cfe = new ConditionalFormattingEvaluator(wb, formulaEval); + + sheet = wb.getSheetAt(1); + + assertEquals("wrong # of matching rules", 1, getRulesFor(3, 1).size()); + } + + @Test + public void testRangeCondition() throws Exception { + wb = XSSFTestDataSamples.openSampleWorkbook("conditional_formatting_multiple_ranges.xlsx"); + formulaEval = new XSSFFormulaEvaluator(wb); + cfe = new ConditionalFormattingEvaluator(wb, formulaEval); + + sheet = wb.getSheetAt(0); + + assertEquals("wrong # of matching rules", 0, getRulesFor(0, 0).size()); + assertEquals("wrong # of matching rules", 0, getRulesFor(1, 0).size()); + assertEquals("wrong # of matching rules", 0, getRulesFor(2, 0).size()); + assertEquals("wrong # of matching rules", 1, getRulesFor(3, 0).size()); + assertEquals("wrong # of matching rules", 0, getRulesFor(0, 1).size()); + assertEquals("wrong # of matching rules", 0, getRulesFor(1, 1).size()); + assertEquals("wrong # of matching rules", 1, getRulesFor(2, 1).size()); + assertEquals("wrong # of matching rules", 1, getRulesFor(3, 1).size()); + assertEquals("wrong # of matching rules", 1, getRulesFor(0, 3).size()); + assertEquals("wrong # of matching rules", 0, getRulesFor(1, 3).size()); + assertEquals("wrong # of matching rules", 1, getRulesFor(2, 3).size()); + assertEquals("wrong # of matching rules", 0, getRulesFor(0, 6).size()); + assertEquals("wrong # of matching rules", 0, getRulesFor(3, 6).size()); + assertEquals("wrong # of matching rules", 0, getRulesFor(2, 6).size()); + } + private List getRulesFor(int row, int col) { ref = new CellReference(sheet.getSheetName(), row, col, false, false); return rules = cfe.getConditionalFormattingForCell(ref); 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 e4f47be46..0f6107279 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFFormulaEvaluation.java @@ -428,4 +428,17 @@ public final class TestXSSFFormulaEvaluation extends BaseTestFormulaEvaluator { CellValue value = evaluator.evaluate(cell); assertEquals(3750, value.getNumberValue(), 0.001); } + + @Test + public void testBug61495() { + Workbook wb = XSSFTestDataSamples.openSampleWorkbook("61495-test.xlsm"); + FormulaEvaluator evaluator = wb.getCreationHelper().createFormulaEvaluator(); + Cell cell = wb.getSheetAt(0).getRow(0).getCell(1); +// assertEquals("D 67.10", cell.getStringCellValue()); + + CellValue value = evaluator.evaluate(cell); + assertEquals("D 67.10", value.getStringValue()); + + assertEquals("D 0,068", evaluator.evaluate(wb.getSheetAt(0).getRow(1).getCell(1))); + } } diff --git a/test-data/spreadsheet/61495-test.xlsm b/test-data/spreadsheet/61495-test.xlsm new file mode 100644 index 000000000..89efea5ab Binary files /dev/null and b/test-data/spreadsheet/61495-test.xlsm differ diff --git a/test-data/spreadsheet/conditional_formatting_cell_is.xlsx b/test-data/spreadsheet/conditional_formatting_cell_is.xlsx new file mode 100644 index 000000000..0177bddf6 Binary files /dev/null and b/test-data/spreadsheet/conditional_formatting_cell_is.xlsx differ diff --git a/test-data/spreadsheet/conditional_formatting_multiple_ranges.xlsx b/test-data/spreadsheet/conditional_formatting_multiple_ranges.xlsx new file mode 100644 index 000000000..43a67d93f Binary files /dev/null and b/test-data/spreadsheet/conditional_formatting_multiple_ranges.xlsx differ diff --git a/test-data/spreadsheet/test_conditional_formatting.xlsx b/test-data/spreadsheet/test_conditional_formatting.xlsx new file mode 100644 index 000000000..5ed82e10f Binary files /dev/null and b/test-data/spreadsheet/test_conditional_formatting.xlsx differ