From 1253a295710ed82680dd5153e868edd87a5a981d Mon Sep 17 00:00:00 2001 From: Vladislav Galas Date: Sat, 26 Jan 2019 10:09:13 +0000 Subject: [PATCH] setCellFormula: blank cell gets value 0, non-blank value is preserved git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1852212 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/hssf/usermodel/HSSFCell.java | 47 +++- .../org/apache/poi/ss/usermodel/Cell.java | 13 +- .../org/apache/poi/ss/usermodel/CellBase.java | 33 ++- .../apache/poi/xssf/streaming/SXSSFCell.java | 213 +++++++++++------- .../apache/poi/xssf/usermodel/XSSFCell.java | 4 +- .../poi/xssf/streaming/TestSXSSFCell.java | 11 +- .../apache/poi/ss/usermodel/BaseTestCell.java | 54 +++++ 7 files changed, 268 insertions(+), 107 deletions(-) diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java index ee4d828d5e..dbeaba8b48 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java @@ -46,8 +46,10 @@ import org.apache.poi.ss.usermodel.Cell; import org.apache.poi.ss.usermodel.CellBase; import org.apache.poi.ss.usermodel.CellStyle; import org.apache.poi.ss.usermodel.CellType; +import org.apache.poi.ss.usermodel.CellValue; import org.apache.poi.ss.usermodel.Comment; import org.apache.poi.ss.usermodel.FormulaError; +import org.apache.poi.ss.usermodel.FormulaEvaluator; import org.apache.poi.ss.usermodel.Hyperlink; import org.apache.poi.ss.usermodel.RichTextString; import org.apache.poi.ss.util.CellAddress; @@ -290,8 +292,9 @@ public class HSSFCell extends CellBase { frec.setRow(row); frec.setColumn(col); } - if (setValue) - { + if (getCellType() == CellType.BLANK) { + frec.getFormulaRecord().setValue(0); + } else if (setValue) { frec.getFormulaRecord().setValue(getNumericCellValue()); } frec.setXFIndex(styleIndex); @@ -596,6 +599,7 @@ public class HSSFCell extends CellBase { short col=_record.getColumn(); short styleIndex=_record.getXFIndex(); + final CellValue savedValue = readValue(); int sheetIndex = _book.getSheetIndex(_sheet); Ptg[] ptgs = HSSFFormulaParser.parse(formula, _book, FormulaType.CELL, sheetIndex); setCellType(CellType.FORMULA, false, row, col, styleIndex); @@ -603,13 +607,48 @@ public class HSSFCell extends CellBase { FormulaRecord frec = agg.getFormulaRecord(); frec.setOptions((short) 2); - frec.setValue(0); - //only set to default if there is no extended format index already set if (agg.getXFIndex() == (short)0) { agg.setXFIndex((short) 0x0f); } agg.setParsedExpression(ptgs); + + restoreValue(savedValue); + } + + private CellValue readValue() { + final CellType valueType = getCellType() == CellType.FORMULA ? getCachedFormulaResultType() : getCellType(); + switch (valueType) { + case NUMERIC: + return new CellValue(getNumericCellValue()); + case STRING: + return new CellValue(getStringCellValue()); + case BOOLEAN: + return CellValue.valueOf(getBooleanCellValue()); + case ERROR: + return CellValue.getError(getErrorCellValue()); + default: + throw new IllegalStateException(); + } + } + + private void restoreValue(CellValue value) { + switch (value.getCellType()) { + case NUMERIC: + setCellValue(value.getNumberValue()); + break; + case STRING: + setCellValue(value.getStringValue()); + break; + case BOOLEAN: + setCellValue(value.getBooleanValue()); + break; + case ERROR: + setCellErrorValue(FormulaError.forInt(value.getErrorValue())); + break; + default: + throw new IllegalStateException(); + } } @Override diff --git a/src/java/org/apache/poi/ss/usermodel/Cell.java b/src/java/org/apache/poi/ss/usermodel/Cell.java index b255c03abe..6f960a30d7 100644 --- a/src/java/org/apache/poi/ss/usermodel/Cell.java +++ b/src/java/org/apache/poi/ss/usermodel/Cell.java @@ -198,19 +198,18 @@ public interface Cell { /** * Sets formula for this cell. + *

If {@code formula} is not null, sets or updates the formula. If {@code formula} is null, removes the formula. + * Or use {@link #removeFormula()} to remove the formula.

* - *

- * Note, this method only sets the formula string and does not calculate the formula value. - * To set the precalculated value use {@link #setCellValue} - *

+ *

Note, this method only sets the formula string and does not calculate the formula value. + * To set the precalculated value use {@link #setCellValue}.

* - *

- * If the cell was blank, sets value to 0. Otherwise, preserves the value as precalculated. - *

+ *

If the cell was blank, sets value to 0. Otherwise, preserves the value as precalculated.

* * @param formula the formula to set, e.g. "SUM(C4:E4)". * If the argument is null then the current formula is removed. * + * @see Cell#removeFormula * @throws IllegalStateException if this cell is a part of an array formula group containing other cells * @throws FormulaParseException if the formula has incorrect syntax or is otherwise invalid */ diff --git a/src/java/org/apache/poi/ss/usermodel/CellBase.java b/src/java/org/apache/poi/ss/usermodel/CellBase.java index 5a7e1f282a..7e3929b236 100644 --- a/src/java/org/apache/poi/ss/usermodel/CellBase.java +++ b/src/java/org/apache/poi/ss/usermodel/CellBase.java @@ -82,29 +82,46 @@ public abstract class CellBase implements Cell { */ @Override public final void setCellFormula(String formula) throws FormulaParseException, IllegalStateException { + // todo validate formula here, before changing the cell? + tryToDeleteArrayFormulaIfSet(); + if (formula == null) { removeFormula(); return; } - CellType previousValueType = getCellType() == CellType.FORMULA ? getCachedFormulaResultType() : getCellType(); - - tryToDeleteArrayFormulaIfSet(); - - setCellFormulaImpl(formula); - - if (previousValueType == CellType.BLANK) { + // 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); } /** - * Implementation-specific setting the formula. + * Implementation-specific setting the formula. Formula is not null. * Shall not change the value. * @param formula */ protected abstract void setCellFormulaImpl(String formula); + /** + * Get value type of this cell. Can return BLANK, NUMERIC, STRING, BOOLEAN or ERROR. + * For current implementations where type is strongly coupled with formula, is equivalent to + * getCellType() == CellType.FORMULA ? getCachedFormulaResultType() : getCellType() + * + *

This is meant as a temporary helper method until the time when value type is decoupled from the formula.

+ * @return value type + */ + protected final CellType getValueType() { + CellType type = getCellType(); + if (type != CellType.FORMULA) { + return type; + } + return getCachedFormulaResultType(); + } + /** * {@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 29b4b2652b..3f67266f21 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFCell.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFCell.java @@ -55,6 +55,7 @@ public class SXSSFCell extends CellBase { public SXSSFCell(SXSSFRow row, CellType cellType) { _row=row; + _value = new BlankValue(); setType(cellType); } @@ -326,15 +327,37 @@ public class SXSSFCell extends CellBase { * @throws FormulaParseException if the formula has incorrect syntax or is otherwise invalid */ @Override - public void setCellFormulaImpl(String formula) throws FormulaParseException - { - if(formula == null) { - setType(CellType.BLANK); - return; + public void setCellFormulaImpl(String formula) throws FormulaParseException { + assert formula != null; + if (getCellType() == CellType.FORMULA) { + ((FormulaValue)_value).setValue(formula); + } else { + switch (getCellType()) { + case NUMERIC: + _value = new NumericFormulaValue(formula, getNumericCellValue()); + break; + case STRING: + if (_value instanceof PlainStringValue) { + _value = new StringFormulaValue(formula, getStringCellValue()); + } else { + assert(_value instanceof RichTextValue); + _value = new RichTextStringFormulaValue(formula, ((RichTextValue) _value).getValue()); + } + break; + case BOOLEAN: + _value = new BooleanFormulaValue(formula, getBooleanCellValue()); + break; + case ERROR: + _value = new ErrorFormulaValue(formula, getErrorCellValue()); + break; + case _NONE: + // fall-through + case FORMULA: + // fall-through + case BLANK: + throw new AssertionError(); + } } - - ensureFormulaType(computeTypeFromFormula(formula)); - ((FormulaValue)_value).setValue(formula); } @Override @@ -531,14 +554,9 @@ public class SXSSFCell extends CellBase { public void setCellErrorValue(byte value) { // for formulas, we want to keep the type and only have an ERROR as formula value if(_value.getType()==CellType.FORMULA) { - // ensure that the type is "ERROR" - setFormulaType(CellType.ERROR); - - // populate the value - ((ErrorFormulaValue) _value).setPreEvaluatedValue(value); + _value = new ErrorFormulaValue(getCellFormula(), value); } else { - ensureTypeOrFormulaType(CellType.ERROR); - ((ErrorValue) _value).setValue(value); + _value = new ErrorValue(value); } } @@ -870,8 +888,7 @@ public class SXSSFCell extends CellBase { // don't change cell type for formulas if(_value.getType() == CellType.FORMULA) { String formula = ((FormulaValue)_value).getValue(); - _value = new RichTextStringFormulaValue(); - ((RichTextStringFormulaValue) _value).setValue(formula); + _value = new RichTextStringFormulaValue(formula, new XSSFRichTextString("")); } else if(_value.getType()!=CellType.STRING || !((StringValue)_value).isRichText()) { _value = new RichTextValue(); @@ -882,12 +899,7 @@ public class SXSSFCell extends CellBase { if(_value.getType()!=type) setType(type); } - /*package*/ void ensureFormulaType(CellType type) - { - if(_value.getType()!=CellType.FORMULA - ||((FormulaValue)_value).getFormulaType()!=type) - setFormulaType(type); - } + /* * Sets the cell type to type if it is different */ @@ -903,7 +915,22 @@ public class SXSSFCell extends CellBase { { if(((FormulaValue)_value).getFormulaType()==type) return; - setFormulaType(type); // once a formula, always a formula + switch (type) { + case BOOLEAN: + _value = new BooleanFormulaValue(getCellFormula(), false); + break; + case NUMERIC: + _value = new NumericFormulaValue(getCellFormula(), 0); + break; + case STRING: + _value = new StringFormulaValue(getCellFormula(), ""); + break; + case ERROR: + _value = new ErrorFormulaValue(getCellFormula(), FormulaError._NO_ERROR.getCode()); + break; + default: + throw new AssertionError(); + } return; } setType(type); @@ -937,7 +964,9 @@ public class SXSSFCell extends CellBase { } case FORMULA: { - _value = new NumericFormulaValue(); + if (getCellType() == CellType.BLANK) { + _value = new NumericFormulaValue("", 0); + } break; } case BLANK: @@ -967,49 +996,6 @@ public class SXSSFCell extends CellBase { } } } - /*package*/ void setFormulaType(CellType type) - { - Value prevValue = _value; - switch(type) - { - case NUMERIC: - { - _value = new NumericFormulaValue(); - break; - } - case STRING: - { - _value = new StringFormulaValue(); - break; - } - case BOOLEAN: - { - _value = new BooleanFormulaValue(); - break; - } - case ERROR: - { - _value = new ErrorFormulaValue(); - break; - } - default: - { - throw new IllegalArgumentException("Illegal type " + type); - } - } - - // if we had a Formula before, we should copy over the _value of the formula - if(prevValue instanceof FormulaValue) { - ((FormulaValue)_value)._value = ((FormulaValue)prevValue)._value; - } - } - -//TODO: implement this correctly - @NotImplemented - /*package*/ CellType computeTypeFromFormula(String formula) - { - return CellType.NUMERIC; - } //COPIED FROM https://svn.apache.org/repos/asf/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java since the functions are declared private there /** * Used to help format error messages @@ -1123,9 +1109,17 @@ public class SXSSFCell extends CellBase { { CellType getType(); } - static class NumericValue implements Value - { + static class NumericValue implements Value { double _value; + + public NumericValue() { + _value = 0; + } + + public NumericValue(double _value) { + this._value = _value; + } + public CellType getType() { return CellType.NUMERIC; @@ -1190,6 +1184,11 @@ public class SXSSFCell extends CellBase { static abstract class FormulaValue implements Value { String _value; + + public FormulaValue(String _value) { + this._value = _value; + } + public CellType getType() { return CellType.FORMULA; @@ -1204,9 +1203,14 @@ public class SXSSFCell extends CellBase { } abstract CellType getFormulaType(); } - static class NumericFormulaValue extends FormulaValue - { + static class NumericFormulaValue extends FormulaValue { double _preEvaluatedValue; + + public NumericFormulaValue(String formula, double _preEvaluatedValue) { + super(formula); + this._preEvaluatedValue = _preEvaluatedValue; + } + @Override CellType getFormulaType() { @@ -1221,9 +1225,14 @@ public class SXSSFCell extends CellBase { return _preEvaluatedValue; } } - static class StringFormulaValue extends FormulaValue - { + static class StringFormulaValue extends FormulaValue { String _preEvaluatedValue; + + public StringFormulaValue(String formula, String value) { + super(formula); + _preEvaluatedValue = value; + } + @Override CellType getFormulaType() { @@ -1238,9 +1247,15 @@ public class SXSSFCell extends CellBase { return _preEvaluatedValue; } } - static class RichTextStringFormulaValue extends FormulaValue - { + + static class RichTextStringFormulaValue extends FormulaValue { RichTextString _preEvaluatedValue; + + public RichTextStringFormulaValue(String formula, RichTextString value) { + super(formula); + _preEvaluatedValue = value; + } + @Override CellType getFormulaType() { @@ -1255,9 +1270,15 @@ public class SXSSFCell extends CellBase { return _preEvaluatedValue; } } - static class BooleanFormulaValue extends FormulaValue - { + + static class BooleanFormulaValue extends FormulaValue { boolean _preEvaluatedValue; + + public BooleanFormulaValue(String formula, boolean value) { + super(formula); + _preEvaluatedValue = value; + } + @Override CellType getFormulaType() { @@ -1272,9 +1293,15 @@ public class SXSSFCell extends CellBase { return _preEvaluatedValue; } } - static class ErrorFormulaValue extends FormulaValue - { + + static class ErrorFormulaValue extends FormulaValue { byte _preEvaluatedValue; + + public ErrorFormulaValue(String formula, byte value) { + super(formula); + _preEvaluatedValue = value; + } + @Override CellType getFormulaType() { @@ -1289,16 +1316,25 @@ public class SXSSFCell extends CellBase { return _preEvaluatedValue; } } - static class BlankValue implements Value - { + + static class BlankValue implements Value { public CellType getType() { return CellType.BLANK; } } - static class BooleanValue implements Value - { + + static class BooleanValue implements Value { boolean _value; + + public BooleanValue() { + _value = false; + } + + public BooleanValue(boolean _value) { + this._value = _value; + } + public CellType getType() { return CellType.BOOLEAN; @@ -1315,6 +1351,15 @@ public class SXSSFCell extends CellBase { static class ErrorValue implements Value { byte _value; + + public ErrorValue() { + _value = FormulaError._NO_ERROR.getCode(); + } + + public ErrorValue(byte _value) { + this._value = _value; + } + public CellType getType() { return CellType.ERROR; diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java index 47ccb646bc..9eadae5b26 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java @@ -551,6 +551,7 @@ public final class XSSFCell extends CellBase { */ @Override protected void setCellFormulaImpl(String formula) { + assert formula != null; setFormula(formula, FormulaType.CELL); } @@ -590,9 +591,6 @@ public final class XSSFCell extends CellBase { f.setStringValue(formula); _cell.setF(f); } - if(_cell.isSetV()) { - _cell.unsetV(); - } } @Override diff --git a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFCell.java b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFCell.java index 7a030defd9..8ecbce0544 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFCell.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFCell.java @@ -175,7 +175,16 @@ public class TestSXSSFCell extends BaseTestXCell { @Override @Ignore - @Test public void removeFormula_turnsCellToBlank_whenFormulaWasASingleCellArrayFormula() { } + + @Override + @Ignore + public void setCellFormula_onASingleCellArrayFormulaCell_preservesTheValue() { + } + + @Test + @Ignore + public void setCellFormula_isExceptionSafe_onBlankCell() { + } } diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java index 242b4abeec..9afa79f956 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java @@ -32,11 +32,13 @@ import java.util.Date; import java.util.GregorianCalendar; import java.util.Locale; import java.util.TimeZone; +import java.util.function.Consumer; import org.apache.poi.common.usermodel.HyperlinkType; import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.ss.ITestDataProvider; import org.apache.poi.ss.SpreadsheetVersion; +import org.apache.poi.ss.formula.FormulaParseException; import org.apache.poi.ss.util.CellRangeAddress; import org.apache.poi.util.LocaleUtil; import org.junit.Test; @@ -1303,6 +1305,58 @@ public abstract class BaseTestCell { assertEquals(CellType.BLANK, cell.getCellType()); } + @Test + public void setCellFormula_onABlankCell_setsValueToZero() { + Cell cell = getInstance(); + cell.setCellFormula("\"foo\""); + assertEquals(CellType.FORMULA, cell.getCellType()); + assertEquals(CellType.NUMERIC, cell.getCachedFormulaResultType()); + assertEquals(0, cell.getNumericCellValue(), 0); + } + + + @Test + public void setCellFormula_onANonBlankCell_preservesTheValue() { + Cell cell = getInstance(); + cell.setCellValue(true); + cell.setCellFormula("\"foo\""); + assertEquals(CellType.FORMULA, cell.getCellType()); + assertEquals(CellType.BOOLEAN, cell.getCachedFormulaResultType()); + assertTrue(cell.getBooleanCellValue()); + } + + @Test + public void setCellFormula_onAFormulaCell_changeFormula_preservesTheValue() { + Cell cell = getInstance(); + cell.setCellFormula("\"foo\""); + cell.setCellValue(true); + assertEquals(CellType.FORMULA, cell.getCellType()); + assertEquals(CellType.BOOLEAN, cell.getCachedFormulaResultType()); + assertTrue(cell.getBooleanCellValue()); + + cell.setCellFormula("\"bar\""); + assertEquals(CellType.FORMULA, cell.getCellType()); + assertEquals(CellType.BOOLEAN, cell.getCachedFormulaResultType()); + assertTrue(cell.getBooleanCellValue()); + } + + @Test + public void setCellFormula_onASingleCellArrayFormulaCell_preservesTheValue() { + Cell cell = getInstance(); + cell.getSheet().setArrayFormula("\"foo\"", CellRangeAddress.valueOf("A1")); + cell.setCellValue(true); + + assertTrue(cell.isPartOfArrayFormulaGroup()); + assertEquals(CellType.FORMULA, cell.getCellType()); + assertEquals(CellType.BOOLEAN, cell.getCachedFormulaResultType()); + assertTrue(cell.getBooleanCellValue()); + + cell.getSheet().setArrayFormula("\"bar\"", CellRangeAddress.valueOf("A1")); + assertEquals(CellType.FORMULA, cell.getCellType()); + assertEquals(CellType.BOOLEAN, cell.getCachedFormulaResultType()); + assertTrue(cell.getBooleanCellValue()); + } + private Cell getInstance() { return _testDataProvider.createWorkbook().createSheet().createRow(0).createCell(0); } -- 2.39.5