Bug 63339: Use removeFormulaImpl() where possible and add a test which verifies that setting a string-value on a formula cell is performed

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1889837 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Dominik Stadler 2021-05-13 12:11:26 +00:00
parent 5e656e05ef
commit 454d0452ef
2 changed files with 53 additions and 24 deletions

View File

@ -138,9 +138,9 @@ public final class XSSFCell extends CellBase {
/** /**
* Copy cell value, formula and style, from srcCell per cell copy policy * Copy cell value, formula and style, from srcCell per cell copy policy
* If srcCell is null, clears the cell value and cell style per cell copy policy * If srcCell is null, clears the cell value and cell style per cell copy policy
* *
* This does not shift references in formulas. Use {@link org.apache.poi.xssf.usermodel.helpers.XSSFRowShifter} to shift references in formulas. * This does not shift references in formulas. Use {@link org.apache.poi.xssf.usermodel.helpers.XSSFRowShifter} to shift references in formulas.
* *
* @param srcCell The cell to take value, formula and style from * @param srcCell The cell to take value, formula and style from
* @param policy The policy for copying the information, see {@link CellCopyPolicy} * @param policy The policy for copying the information, see {@link CellCopyPolicy}
* @throws IllegalArgumentException if copy cell style and srcCell is from a different workbook * @throws IllegalArgumentException if copy cell style and srcCell is from a different workbook
@ -190,12 +190,12 @@ public final class XSSFCell extends CellBase {
setBlank(); setBlank();
} }
} }
// Copy CellStyle // Copy CellStyle
if (policy.isCopyCellStyle()) { if (policy.isCopyCellStyle()) {
setCellStyle(srcCell == null ? null : srcCell.getCellStyle()); setCellStyle(srcCell == null ? null : srcCell.getCellStyle());
} }
final Hyperlink srcHyperlink = (srcCell == null) ? null : srcCell.getHyperlink(); final Hyperlink srcHyperlink = (srcCell == null) ? null : srcCell.getHyperlink();
if (policy.isMergeHyperlink()) { if (policy.isMergeHyperlink()) {
@ -444,7 +444,7 @@ public final class XSSFCell extends CellBase {
// existing behavior - create a new XSSFEvaluationWorkbook for every call // existing behavior - create a new XSSFEvaluationWorkbook for every call
return getCellFormula(null); return getCellFormula(null);
} }
/** /**
* package/hierarchy use only - reuse an existing evaluation workbook if available for caching * package/hierarchy use only - reuse an existing evaluation workbook if available for caching
* *
@ -513,7 +513,7 @@ public final class XSSFCell extends CellBase {
* To set the precalculated value use {@link #setCellValue(double)} or {@link #setCellValue(String)} * To set the precalculated value use {@link #setCellValue(double)} or {@link #setCellValue(String)}
* </p> * </p>
* <p> * <p>
* Note, if there are any shared formulas, his will invalidate any * Note, if there are any shared formulas, his will invalidate any
* {@link FormulaEvaluator} instances based on this workbook. * {@link FormulaEvaluator} instances based on this workbook.
* </p> * </p>
* *
@ -539,11 +539,7 @@ public final class XSSFCell extends CellBase {
private void setFormula(String formula, FormulaType formulaType) { private void setFormula(String formula, FormulaType formulaType) {
XSSFWorkbook wb = _row.getSheet().getWorkbook(); XSSFWorkbook wb = _row.getSheet().getWorkbook();
if (formulaType == FormulaType.ARRAY && formula == null) { if (formulaType == FormulaType.ARRAY && formula == null) {
wb.onDeleteFormula(this); removeFormulaImpl();
if (_cell.isSetF()) {
_row.getSheet().onDeleteFormula(this, null);
_cell.unsetF();
}
return; return;
} }
@ -630,7 +626,7 @@ public final class XSSFCell extends CellBase {
* *
* <p>To change the style of a cell without affecting other cells that use the same style, * <p>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, java.util.Map)}</p> * use {@link org.apache.poi.ss.util.CellUtil#setCellStyleProperties(Cell, java.util.Map)}</p>
* *
* @param style reference contained in the workbook. * @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. * If the value is null then the style information is removed causing the cell to used the default workbook style.
* @throws IllegalArgumentException if style belongs to a different styles source (most likely because style is from a different Workbook) * @throws IllegalArgumentException if style belongs to a different styles source (most likely because style is from a different Workbook)
@ -649,7 +645,7 @@ public final class XSSFCell extends CellBase {
_cell.setS(idx); _cell.setS(idx);
} }
} }
/** /**
* POI currently supports these formula types: * POI currently supports these formula types:
* <ul> * <ul>
@ -666,7 +662,7 @@ public final class XSSFCell extends CellBase {
} }
return false; return false;
} }
/** /**
* Return the cell type. Tables in an array formula return * Return the cell type. Tables in an array formula return
* {@link CellType#FORMULA} for all cells, even though the formula is only defined * {@link CellType#FORMULA} for all cells, even though the formula is only defined
@ -902,7 +898,7 @@ public final class XSSFCell extends CellBase {
protected void setCellTypeImpl(CellType cellType) { protected void setCellTypeImpl(CellType cellType) {
setCellType(cellType, null); setCellType(cellType, null);
} }
/** /**
* Needed by bug #62834, which points out getCellFormula() expects an evaluation context or creates a new one, * Needed by bug #62834, which points out getCellFormula() expects an evaluation context or creates a new one,
* so if there is one in use, it needs to be carried on through. * so if there is one in use, it needs to be carried on through.
@ -1125,10 +1121,10 @@ public final class XSSFCell extends CellBase {
public CTCell getCTCell(){ public CTCell getCTCell(){
return _cell; return _cell;
} }
/** /**
* Set a new internal xml bean. This is only for internal use, do not call this from outside! * Set a new internal xml bean. This is only for internal use, do not call this from outside!
* *
* This is necessary in some rare cases to work around XMLBeans specialties. * This is necessary in some rare cases to work around XMLBeans specialties.
*/ */
@Internal @Internal
@ -1168,7 +1164,7 @@ public final class XSSFCell extends CellBase {
// fall-through // fall-through
case BLANK: case BLANK:
return false; return false;
default: default:
throw new IllegalStateException("Unexpected cell type (" + cellType + ")"); throw new IllegalStateException("Unexpected cell type (" + cellType + ")");
} }
@ -1212,18 +1208,18 @@ public final class XSSFCell extends CellBase {
} }
throw new IllegalStateException("Unexpected boolean cached formula value '" throw new IllegalStateException("Unexpected boolean cached formula value '"
+ textValue + "'."); + textValue + "'.");
case STRING: case STRING:
// fall-through // fall-through
case NUMERIC: case NUMERIC:
// fall-through // fall-through
case ERROR: case ERROR:
return textValue; return textValue;
default: default:
throw new IllegalStateException("Unexpected formula result type (" + cellType + ")"); throw new IllegalStateException("Unexpected formula result type (" + cellType + ")");
} }
} }
@Override @Override
@ -1250,14 +1246,14 @@ public final class XSSFCell extends CellBase {
CalculationChain calcChain = getSheet().getWorkbook().getCalculationChain(); CalculationChain calcChain = getSheet().getWorkbook().getCalculationChain();
int sheetId = Math.toIntExact(getSheet().sheet.getSheetId()); int sheetId = Math.toIntExact(getSheet().sheet.getSheetId());
//remove the reference in the calculation chain //remove the reference in the calculation chain
if(calcChain != null) calcChain.removeItem(sheetId, getReference()); if(calcChain != null) calcChain.removeItem(sheetId, getReference());
CTCell ctCell = getCTCell(); CTCell ctCell = getCTCell();
String r = new CellReference(getRowIndex(), getColumnIndex()).formatAsString(); String r = new CellReference(getRowIndex(), getColumnIndex()).formatAsString();
ctCell.setR(r); ctCell.setR(r);
} }
} }

View File

@ -3498,6 +3498,39 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
} }
} }
@Test
public void testBug63339() 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)");
assertNotNull(((XSSFCell) cell).getCTCell().getF(),
"Element 'f' should contain the formula now");
// this will actually set the "cached" value of the Formula
// you will need to use setCellType() to change the cell to
// a different type of value
cell.setCellValue(34.5);
assertNotNull(((XSSFCell) cell).getCTCell().getF(),
"Element 'f' should not be set now");
assertEquals("34.5", ((XSSFCell) cell).getCTCell().getV(),
"Element 'v' should contain the string now");
try (Workbook wbBack = XSSFTestDataSamples.writeOutAndReadBack(wb)) {
Cell cellBack = wbBack.getSheetAt(0).getRow(0).getCell(0);
assertNotNull(((XSSFCell) cellBack).getCTCell().getF(),
"Element 'f' should not be set now");
assertEquals("34.5", ((XSSFCell) cellBack).getCTCell().getV(),
"Element 'v' should contain the string now");
}
}
}
@Test @Test
void testBug64508() throws IOException { void testBug64508() throws IOException {
try (Workbook wb = XSSFTestDataSamples.openSampleWorkbook("64508.xlsx")) { try (Workbook wb = XSSFTestDataSamples.openSampleWorkbook("64508.xlsx")) {