From cc1b6557ee514aa2f9997b104c42a09aae3400e3 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sun, 29 Mar 2020 08:16:17 +0000 Subject: [PATCH] Bug 63845: Adjust handling of formula-cells to fix regression introduced in 4.1.0 git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1875837 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/hssf/usermodel/HSSFCell.java | 10 +++- .../org/apache/poi/ss/usermodel/CellBase.java | 8 +--- .../apache/poi/xssf/streaming/SXSSFCell.java | 31 ++++++------ .../poi/ss/usermodel/BaseTestXCell.java | 10 ++-- .../poi/xssf/usermodel/TestXSSFBugs.java | 48 +++++++++++++++++++ .../apache/poi/ss/usermodel/BaseTestCell.java | 12 +++-- 6 files changed, 83 insertions(+), 36 deletions(-) diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java index 7b309fe54f..80e066fd91 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java @@ -531,6 +531,12 @@ public class HSSFCell extends CellBase { */ @Override protected void setCellFormulaImpl(String formula) { + // formula cells always have a value. If the cell is blank (either initially or after removing an + // array formula), set value to 0 + if (getValueType() == CellType.BLANK) { + setCellValue(0); + } + assert formula != null; int row=_record.getRow(); @@ -566,7 +572,7 @@ public class HSSFCell extends CellBase { case ERROR: return CellValue.getError(getErrorCellValue()); default: - throw new IllegalStateException(); + throw new IllegalStateException("Unexpected cell-type " + valueType); } } @@ -585,7 +591,7 @@ public class HSSFCell extends CellBase { setCellErrorValue(FormulaError.forInt(value.getErrorValue())); break; default: - throw new IllegalStateException(); + throw new IllegalStateException("Unexpected cell-type " + value.getCellType() + " for cell-value: " + value); } } diff --git a/src/java/org/apache/poi/ss/usermodel/CellBase.java b/src/java/org/apache/poi/ss/usermodel/CellBase.java index 819c789ca8..5cbc17eca3 100644 --- a/src/java/org/apache/poi/ss/usermodel/CellBase.java +++ b/src/java/org/apache/poi/ss/usermodel/CellBase.java @@ -123,12 +123,6 @@ public abstract class CellBase implements Cell { return; } - // formula cells always have a value. If the cell is blank (either initially or after removing an - // array formula), set value to 0 - if (getValueType() == CellType.BLANK) { - setCellValue(0); - } - setCellFormulaImpl(formula); } @@ -255,7 +249,7 @@ public abstract class CellBase implements Cell { * @param value the new date to set */ protected abstract void setCellValueImpl(LocalDateTime value); - + /** * {@inheritDoc} */ diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFCell.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFCell.java index 31e1c38631..a00db5fe1c 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFCell.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFCell.java @@ -51,7 +51,7 @@ public class SXSSFCell extends CellBase { private Value _value; private CellStyle _style; private Property _firstProperty; - + public SXSSFCell(SXSSFRow row, CellType cellType) { _row=row; @@ -133,7 +133,7 @@ public class SXSSFCell extends CellBase { return _value.getType(); } - + /** * Only valid for formula cells * @return one of ({@link CellType#NUMERIC}, {@link CellType#STRING}, @@ -194,7 +194,7 @@ public class SXSSFCell extends CellBase { boolean date1904 = getSheet().getWorkbook().isDate1904(); setCellValue(DateUtil.getExcelDate(value, date1904)); } - + /** * {@inheritDoc} */ @@ -249,6 +249,7 @@ public class SXSSFCell extends CellBase { ((FormulaValue)_value).setValue(formula); } else { switch (getCellType()) { + case BLANK: case NUMERIC: _value = new NumericFormulaValue(formula, getNumericCellValue()); break; @@ -266,12 +267,8 @@ public class SXSSFCell extends CellBase { case ERROR: _value = new ErrorFormulaValue(formula, getErrorCellValue()); break; - case _NONE: - // fall-through - case FORMULA: - // fall-through - case BLANK: - throw new AssertionError(); + default: + throw new IllegalStateException("Cannot set a formula for a cell of type " + getCellType()); } } } @@ -334,7 +331,7 @@ public class SXSSFCell extends CellBase { public double getNumericCellValue() { CellType cellType = getCellType(); - switch(cellType) + switch(cellType) { case BLANK: return 0.0; @@ -366,7 +363,7 @@ public class SXSSFCell extends CellBase { public Date getDateCellValue() { CellType cellType = getCellType(); - if (cellType == CellType.BLANK) + if (cellType == CellType.BLANK) { return null; } @@ -434,7 +431,7 @@ public class SXSSFCell extends CellBase { public String getStringCellValue() { CellType cellType = getCellType(); - switch(cellType) + switch(cellType) { case BLANK: return ""; @@ -510,7 +507,7 @@ public class SXSSFCell extends CellBase { public boolean getBooleanCellValue() { CellType cellType = getCellType(); - switch(cellType) + switch(cellType) { case BLANK: return false; @@ -545,7 +542,7 @@ public class SXSSFCell extends CellBase { public byte getErrorCellValue() { CellType cellType = getCellType(); - switch(cellType) + switch(cellType) { case BLANK: return 0; @@ -568,10 +565,10 @@ public class SXSSFCell extends CellBase { /** *

Set the style for the cell. The style should be an CellStyle created/retreived from * the Workbook.

- * + * *

To change the style of a cell without affecting other cells that use the same style, * use {@link org.apache.poi.ss.util.CellUtil#setCellStyleProperties(Cell, Map)}

- * + * * @param style reference contained in the workbook. * If the value is null then the style information is removed causing the cell to used the default workbook style. * @see org.apache.poi.ss.usermodel.Workbook#createCellStyle @@ -964,7 +961,7 @@ public class SXSSFCell extends CellBase { return false; default: throw new RuntimeException("Unexpected cell type (" + cellType + ")"); } - + } private String convertCellValueToString() { CellType cellType = getCellType(); diff --git a/src/ooxml/testcases/org/apache/poi/ss/usermodel/BaseTestXCell.java b/src/ooxml/testcases/org/apache/poi/ss/usermodel/BaseTestXCell.java index 28d2d48936..46d7d6319e 100644 --- a/src/ooxml/testcases/org/apache/poi/ss/usermodel/BaseTestXCell.java +++ b/src/ooxml/testcases/org/apache/poi/ss/usermodel/BaseTestXCell.java @@ -20,8 +20,6 @@ package org.apache.poi.ss.usermodel; import static org.junit.Assert.assertEquals; import java.io.IOException; -import java.util.Calendar; -import java.util.Date; import org.apache.poi.hssf.usermodel.HSSFCell; import org.apache.poi.ss.ITestDataProvider; @@ -30,9 +28,9 @@ import org.apache.poi.xssf.usermodel.XSSFCell; import org.junit.Test; /** - * Class for combined testing of XML-specific functionality of + * Class for combined testing of XML-specific functionality of * {@link XSSFCell} and {@link SXSSFCell}. - * + * * Any test that is applicable for {@link HSSFCell} as well should go into * the common base class {@link BaseTestCell}. */ @@ -47,14 +45,14 @@ public abstract class BaseTestXCell extends BaseTestCell { Sheet sh = wb1.createSheet(); Row row = sh.createRow(0); Cell cell = row.createCell(0); - String sval = "\u0000\u0002\u0012<>\t\n\u00a0 &\"POI\'\u2122"; + String sval = "\u0000\u0002\u0012<>\t\n\u00a0 &\"POI'\u2122"; cell.setCellValue(sval); Workbook wb2 = _testDataProvider.writeOutAndReadBack(wb1); wb1.close(); // invalid characters are replaced with question marks - assertEquals("???<>\t\n\u00a0 &\"POI\'\u2122", wb2.getSheetAt(0).getRow(0).getCell(0).getStringCellValue()); + assertEquals("???<>\t\n\u00a0 &\"POI'\u2122", wb2.getSheetAt(0).getRow(0).getCell(0).getStringCellValue()); wb2.close(); } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java index 86aa1ccb59..067df075d0 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java @@ -3494,4 +3494,52 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues { LOG.log(POILogger.INFO, Duration.between(start, Instant.now())); } } + + @Test + public void testBug63845() throws IOException { + try (Workbook wb = new XSSFWorkbook()) { + Sheet sheet = wb.createSheet(); + Row row = sheet.createRow(0); + + Cell cell = row.createCell(0, CellType.FORMULA); + cell.setCellFormula("SUM(B1:E1)"); + + assertNull("Element 'v' should not be set for formulas unless the value was calculated", + ((XSSFCell) cell).getCTCell().getV()); + + try (Workbook wbBack = XSSFTestDataSamples.writeOutAndReadBack(wb)) { + Cell cellBack = wbBack.getSheetAt(0).getRow(0).getCell(0); + assertNull("Element 'v' should not be set for formulas unless the value was calculated", + ((XSSFCell) cellBack).getCTCell().getV()); + + wbBack.getCreationHelper().createFormulaEvaluator().evaluateInCell(cellBack); + + assertEquals("Element 'v' should be set now as the formula was calculated manually", + "0.0", ((XSSFCell) cellBack).getCTCell().getV()); + } + } + } + + @Test + public void testBug63845_2() throws IOException { + try (Workbook wb = new XSSFWorkbook()) { + Sheet sheet = wb.createSheet("test"); + Row row = sheet.createRow(0); + row.createCell(0).setCellValue(2); + row.createCell(1).setCellValue(5); + row.createCell(2).setCellFormula("A1+B1"); + + try (Workbook wbBack = XSSFTestDataSamples.writeOutAndReadBack(wb)) { + Cell cellBack = wbBack.getSheetAt(0).getRow(0).getCell(2); + + assertNull("Element 'v' should not be set for formulas unless the value was calculated", + ((XSSFCell) cellBack).getCTCell().getV()); + + wbBack.getCreationHelper().createFormulaEvaluator().evaluateInCell(cellBack); + + assertEquals("Element 'v' should be set now as the formula was calculated manually", + "7.0", ((XSSFCell) cellBack).getCTCell().getV()); + } + } + } } diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java index e6913c19a0..2ab0f48b5e 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java @@ -1352,7 +1352,8 @@ public abstract class BaseTestCell { cell.setCellFormula("\"foo\""); assertEquals(CellType.FORMULA, cell.getCellType()); assertEquals(CellType.BOOLEAN, cell.getCachedFormulaResultType()); - assertTrue(cell.getBooleanCellValue()); + assertTrue("Expected a boolean cell-value, but had 'false'", + cell.getBooleanCellValue()); } } @@ -1369,7 +1370,8 @@ public abstract class BaseTestCell { cell.setCellFormula("\"bar\""); assertEquals(CellType.FORMULA, cell.getCellType()); assertEquals(CellType.BOOLEAN, cell.getCachedFormulaResultType()); - assertTrue(cell.getBooleanCellValue()); + assertTrue("Expected a boolean cell-value, but had 'false'", + cell.getBooleanCellValue()); } } @@ -1387,8 +1389,10 @@ public abstract class BaseTestCell { cell.getSheet().setArrayFormula("\"bar\"", CellRangeAddress.valueOf("A1")); assertEquals(CellType.FORMULA, cell.getCellType()); - assertEquals(CellType.BOOLEAN, cell.getCachedFormulaResultType()); - assertTrue(cell.getBooleanCellValue()); + assertEquals("Expected a boolean cell-value, but had " + cell.getCachedFormulaResultType(), + CellType.BOOLEAN, cell.getCachedFormulaResultType()); + assertTrue("Expected a boolean cell-value, but had 'false'", + cell.getBooleanCellValue()); } }