diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index d138d1503..053650741 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,11 +37,12 @@ - 45031- added implementation for CHOOSE() function + 46479 - fixed bugs related to cached formula values and HSSFFormulaEvaluator.evaluateInCell() + 45031 - added implementation for CHOOSE() function 46361 - resolve licensing issues around the HDGF resource file, chunks_parse_cmds.tbl 46410 - added implementation for TIME() function 46320 - added HSSFPictureData.getFormat() - 46445 fixed HSSFSheet.shiftRow to move hyperlinks + 46445 - fixed HSSFSheet.shiftRow to move hyperlinks fixed formula parser to correctly resolve sheet-level names 46433 - support for shared formulas in XSSF 46299 - support for carriage return and line break in XWPFRun diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 4f1aafec4..6e72711b7 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,11 +34,12 @@ - 45031- added implementation for CHOOSE() function + 46479 - fixed bugs related to cached formula values and HSSFFormulaEvaluator.evaluateInCell() + 45031 - added implementation for CHOOSE() function 46361 - resolve licensing issues around the HDGF resource file, chunks_parse_cmds.tbl 46410 - added implementation for TIME() function 46320 - added HSSFPictureData.getFormat() - 46445 fixed HSSFSheet.shiftRow to move hyperlinks + 46445 - fixed HSSFSheet.shiftRow to move hyperlinks fixed formula parser to correctly resolve sheet-level names 46433 - support for shared formulas in XSSF 46299 - support for carriage return and line break in XWPFRun diff --git a/src/java/org/apache/poi/hssf/record/FormulaRecord.java b/src/java/org/apache/poi/hssf/record/FormulaRecord.java index dd15cfcc1..53ad71a51 100644 --- a/src/java/org/apache/poi/hssf/record/FormulaRecord.java +++ b/src/java/org/apache/poi/hssf/record/FormulaRecord.java @@ -122,7 +122,7 @@ public final class FormulaRecord extends StandardRecord implements CellValueReco return create(STRING, 0); } public static SpecialCachedValue createCachedBoolean(boolean b) { - return create(BOOLEAN, b ? 0 : 1); + return create(BOOLEAN, b ? 1 : 0); } public static SpecialCachedValue createCachedErrorCode(int errorCode) { return create(ERROR_CODE, errorCode); diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java index 2e7431bcc..1d2cac679 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java @@ -470,7 +470,7 @@ public class HSSFCell implements Cell { switch (cellType) { default: setCellType(CELL_TYPE_NUMERIC, false, row, col, styleIndex); - case CELL_TYPE_ERROR: + case CELL_TYPE_NUMERIC: (( NumberRecord ) record).setValue(value); break; case CELL_TYPE_FORMULA: @@ -745,7 +745,7 @@ public class HSSFCell implements Cell { switch (cellType) { default: setCellType(CELL_TYPE_BOOLEAN, false, row, col, styleIndex); - case CELL_TYPE_ERROR: + case CELL_TYPE_BOOLEAN: (( BoolErrRecord ) record).setValue(value); break; case CELL_TYPE_FORMULA: @@ -797,10 +797,13 @@ public class HSSFCell implements Cell { case CELL_TYPE_NUMERIC: return ((NumberRecord)record).getValue() != 0; - // All other cases convert to false - // These choices are not well justified. case CELL_TYPE_FORMULA: - // should really evaluate, but HSSFCell can't call HSSFFormulaEvaluator + // use cached formula result if it's the right type: + FormulaRecord fr = ((FormulaRecordAggregate)record).getFormulaRecord(); + checkFormulaCachedValueType(CELL_TYPE_BOOLEAN, fr); + return fr.getCachedBooleanValue(); + // Other cases convert to false + // These choices are not well justified. case CELL_TYPE_ERROR: case CELL_TYPE_BLANK: return false; diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java b/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java index 1f65588ca..aa1a0a5d0 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java @@ -215,8 +215,8 @@ public class HSSFFormulaEvaluator implements FormulaEvaluator { HSSFCell result = (HSSFCell) cell; if (cell.getCellType() == HSSFCell.CELL_TYPE_FORMULA) { CellValue cv = evaluateFormulaCellValue(cell); - setCellType(cell, cv); // cell will no longer be a formula cell setCellValue(cell, cv); + setCellType(cell, cv); // cell will no longer be a formula cell } return result; } diff --git a/src/testcases/org/apache/poi/hssf/record/TestFormulaRecord.java b/src/testcases/org/apache/poi/hssf/record/TestFormulaRecord.java index b84ffc4c1..517865155 100644 --- a/src/testcases/org/apache/poi/hssf/record/TestFormulaRecord.java +++ b/src/testcases/org/apache/poi/hssf/record/TestFormulaRecord.java @@ -17,6 +17,7 @@ package org.apache.poi.hssf.record; +import junit.framework.AssertionFailedError; import junit.framework.TestCase; import org.apache.poi.hssf.record.formula.AttrPtg; @@ -28,8 +29,7 @@ import org.apache.poi.hssf.usermodel.HSSFCell; import org.apache.poi.hssf.usermodel.HSSFErrorConstants; /** - * Tests the serialization and deserialization of the FormulaRecord - * class works correctly. + * Tests for {@link FormulaRecord} * * @author Andrew C. Oliver */ @@ -168,4 +168,26 @@ public final class TestFormulaRecord extends TestCase { RefPtg rp = (RefPtg) ptgs[0]; assertEquals("B$5", rp.toFormulaString()); } + + /** + * Bug noticed while fixing 46479. Operands of conditional operator ( ? : ) were swapped + * inside {@link FormulaRecord} + */ + public void testCachedValue_bug46479() { + FormulaRecord fr0 = new FormulaRecord(); + FormulaRecord fr1 = new FormulaRecord(); + // test some other cached value types + fr0.setValue(3.5); + assertEquals(3.5, fr0.getValue(), 0.0); + fr0.setCachedResultErrorCode(HSSFErrorConstants.ERROR_REF); + assertEquals(HSSFErrorConstants.ERROR_REF, fr0.getCachedErrorValue()); + + fr0.setCachedResultBoolean(false); + fr1.setCachedResultBoolean(true); + if (fr0.getCachedBooleanValue() == true && fr1.getCachedBooleanValue() == false) { + throw new AssertionFailedError("Identified bug 46479c"); + } + assertEquals(false, fr0.getCachedBooleanValue()); + assertEquals(true, fr1.getCachedBooleanValue()); + } } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFCell.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFCell.java index abb9f1b41..456f263a3 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFCell.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFCell.java @@ -422,5 +422,46 @@ public final class TestHSSFCell extends TestCase { } assertEquals("TRUE", cell.getRichStringCellValue().getString()); } + + public void testChangeTypeErrorToNumber_bug46479() { + HSSFCell cell = new HSSFWorkbook().createSheet("Sheet1").createRow(0).createCell(0); + cell.setCellErrorValue((byte)HSSFErrorConstants.ERROR_NAME); + try { + cell.setCellValue(2.5); + } catch (ClassCastException e) { + throw new AssertionFailedError("Identified bug 46479b"); + } + assertEquals(2.5, cell.getNumericCellValue(), 0.0); + } + + public void testChangeTypeErrorToBoolean_bug46479() { + HSSFCell cell = new HSSFWorkbook().createSheet("Sheet1").createRow(0).createCell(0); + cell.setCellErrorValue((byte)HSSFErrorConstants.ERROR_NAME); + cell.setCellValue(true); + try { + cell.getBooleanCellValue(); + } catch (IllegalStateException e) { + if (e.getMessage().equals("Cannot get a boolean value from a error cell")) { + + throw new AssertionFailedError("Identified bug 46479c"); + } + throw e; + } + assertEquals(true, cell.getBooleanCellValue()); + } + + /** + * Test for bug in convertCellValueToBoolean to make sure that formula results get converted + */ + public void testChangeTypeFormulaToBoolean_bug46479() { + HSSFCell cell = new HSSFWorkbook().createSheet("Sheet1").createRow(0).createCell(0); + cell.setCellFormula("1=1"); + cell.setCellValue(true); + cell.setCellType(HSSFCell.CELL_TYPE_BOOLEAN); + if (cell.getBooleanCellValue() == false) { + throw new AssertionFailedError("Identified bug 46479d"); + } + assertEquals(true, cell.getBooleanCellValue()); + } } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFFormulaEvaluator.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFFormulaEvaluator.java index 828a87238..4aba177bf 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFFormulaEvaluator.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFFormulaEvaluator.java @@ -17,6 +17,7 @@ package org.apache.poi.hssf.usermodel; +import junit.framework.AssertionFailedError; import junit.framework.TestCase; import org.apache.poi.hssf.HSSFTestDataSamples; @@ -92,4 +93,32 @@ public final class TestHSSFFormulaEvaluator extends TestCase { HSSFCell cell = sheet.createRow(0).createCell(0); assertNull(fe.evaluate(cell)); } + + /** + * Test for bug due to attempt to convert a cached formula error result to a boolean + */ + public void testUpdateCachedFormulaResultFromErrorToNumber_bug46479() { + + HSSFWorkbook wb = new HSSFWorkbook(); + HSSFSheet sheet = wb.createSheet("Sheet1"); + HSSFRow row = sheet.createRow(0); + HSSFCell cellA1 = row.createCell(0); + HSSFCell cellB1 = row.createCell(1); + cellB1.setCellFormula("A1+1"); + HSSFFormulaEvaluator fe = new HSSFFormulaEvaluator(wb); + + cellA1.setCellErrorValue((byte)HSSFErrorConstants.ERROR_NAME); + fe.evaluateFormulaCell(cellB1); + + cellA1.setCellValue(2.5); + fe.notifyUpdateCell(cellA1); + try { + fe.evaluateInCell(cellB1); + } catch (IllegalStateException e) { + if (e.getMessage().equals("Cannot get a numeric value from a error formula cell")) { + throw new AssertionFailedError("Identified bug 46479a"); + } + } + assertEquals(3.5, cellB1.getNumericCellValue(), 0.0); + } }