From: Dominik Stadler Date: Tue, 19 May 2015 13:13:09 +0000 (+0000) Subject: Prevent problems reported in Bug 56574 by ensuring that Cells are properly removed... X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=589b18915101386d79f59bf9aa78c220b04c5911;p=poi.git Prevent problems reported in Bug 56574 by ensuring that Cells are properly removed when a row is overwritten by calling createRow() with it's rownum. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1680280 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java index 189922ad47..840ed624d8 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java @@ -602,6 +602,15 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet { CTRow ctRow; XSSFRow prev = _rows.get(rownum); if(prev != null){ + // the Cells in an existing row are invalidated on-purpose, in order to clean up correctly, we + // need to call the remove, so things like ArrayFormulas and CalculationChain updates are done + // correctly. + // We remove the cell this way as the internal cell-list is changed by the remove call and + // thus would cause ConcurrentModificationException otherwise + while(prev.getFirstCellNum() != -1) { + prev.removeCell(prev.getCell(prev.getFirstCellNum())); + } + ctRow = prev.getCTRow(); ctRow.set(CTRow.Factory.newInstance()); } else { @@ -2614,6 +2623,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet { } } }); + for (Iterator it = rowIterator() ; it.hasNext() ; ) { XSSFRow row = (XSSFRow)it.next(); 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 5658f4c12a..60f6188a01 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java @@ -34,6 +34,9 @@ import java.io.IOException; import java.util.Arrays; import java.util.Calendar; import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; import org.apache.poi.EncryptedDocumentException; import org.apache.poi.POIDataSamples; @@ -87,6 +90,7 @@ import org.apache.poi.xssf.streaming.SXSSFWorkbook; import org.apache.poi.xssf.usermodel.extensions.XSSFCellFill; import org.junit.Ignore; import org.junit.Test; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCalcCell; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCols; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTDefinedName; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTDefinedNames; @@ -2502,4 +2506,98 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues { wb.close(); tmp.delete(); } + + @Test + public void test56574() throws IOException { + runTest56574(false); + runTest56574(true); + } + + @SuppressWarnings("deprecation") + private void runTest56574(boolean createRow) throws IOException { + Workbook wb = XSSFTestDataSamples.openSampleWorkbook("56574.xlsx"); + + Sheet sheet = wb.getSheet("Func"); + assertNotNull(sheet); + + Map data; + data = new TreeMap(); + data.put("1", new Object[] {"ID", "NAME", "LASTNAME"}); + data.put("2", new Object[] {2, "Amit", "Shukla"}); + data.put("3", new Object[] {1, "Lokesh", "Gupta"}); + data.put("4", new Object[] {4, "John", "Adwards"}); + data.put("5", new Object[] {2, "Brian", "Schultz"}); + + Set keyset = data.keySet(); + int rownum = 1; + for (String key : keyset) + { + final Row row; + if(createRow) { + row = sheet.createRow(rownum++); + } else { + row = sheet.getRow(rownum++); + } + assertNotNull(row); + + Object [] objArr = data.get(key); + int cellnum = 0; + for (Object obj : objArr) + { + Cell cell = row.getCell(cellnum); + if(cell == null){ + cell = row.createCell(cellnum); + } else { + if(cell.getCellType() == Cell.CELL_TYPE_FORMULA) { + cell.setCellFormula(null); + cell.getCellStyle().setDataFormat((short) 0); + } + } + if(obj instanceof String) { + cell.setCellValue((String)obj); + } else if(obj instanceof Integer) { + cell.setCellValue((Integer)obj); + } + cellnum++; + } + } + + XSSFFormulaEvaluator.evaluateAllFormulaCells((XSSFWorkbook) wb); + wb.getCreationHelper().createFormulaEvaluator().evaluateAll(); + + CalculationChain chain = ((XSSFWorkbook)wb).getCalculationChain(); + CTCalcCell[] cArray = chain.getCTCalcChain().getCArray(); + for(CTCalcCell calc : cArray) { + // A2 to A6 should be gone + assertFalse(calc.getR().equals("A2")); + assertFalse(calc.getR().equals("A3")); + assertFalse(calc.getR().equals("A4")); + assertFalse(calc.getR().equals("A5")); + assertFalse(calc.getR().equals("A6")); + } + + /*FileOutputStream out = new FileOutputStream(new File("C:\\temp\\56574.xlsx")); + try { + wb.write(out); + } finally { + out.close(); + }*/ + + Workbook wbBack = XSSFTestDataSamples.writeOutAndReadBack(wb); + Sheet sheetBack = wbBack.getSheet("Func"); + assertNotNull(sheetBack); + + chain = ((XSSFWorkbook)wbBack).getCalculationChain(); + cArray = chain.getCTCalcChain().getCArray(); + for(CTCalcCell calc : cArray) { + // A2 to A6 should be gone + assertFalse(calc.getR().equals("A2")); + assertFalse(calc.getR().equals("A3")); + assertFalse(calc.getR().equals("A4")); + assertFalse(calc.getR().equals("A5")); + assertFalse(calc.getR().equals("A6")); + } + + wb.close(); + } } diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java index fe11df6589..8b6732ca45 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java @@ -18,9 +18,12 @@ package org.apache.poi.ss.usermodel; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import java.io.IOException; import java.util.HashMap; import java.util.Map; @@ -637,7 +640,7 @@ public abstract class BaseTestBugzillaIssues { // Next up, SEARCH on its own cf.setCellFormula("SEARCH(\"am\", A1)"); cf = evaluateCell(wb, cf); - assertEquals(ErrorConstants.ERROR_VALUE, cf.getErrorCellValue()); + assertEquals(FormulaError.VALUE.getCode(), cf.getErrorCellValue()); cf.setCellFormula("SEARCH(\"am\", B1)"); cf = evaluateCell(wb, cf); @@ -645,11 +648,11 @@ public abstract class BaseTestBugzillaIssues { cf.setCellFormula("SEARCH(\"am\", C1)"); cf = evaluateCell(wb, cf); - assertEquals(ErrorConstants.ERROR_VALUE, cf.getErrorCellValue()); + assertEquals(FormulaError.VALUE.getCode(), cf.getErrorCellValue()); cf.setCellFormula("SEARCH(\"am\", D1)"); cf = evaluateCell(wb, cf); - assertEquals(ErrorConstants.ERROR_VALUE, cf.getErrorCellValue()); + assertEquals(FormulaError.VALUE.getCode(), cf.getErrorCellValue()); // Finally, bring it all together @@ -757,4 +760,50 @@ public abstract class BaseTestBugzillaIssues { assertEquals(otherCellText, c1.getStringCellValue()); assertEquals(otherCellText, c2.getStringCellValue()); } + + @Test + public void test56574OverwriteExistingRow() throws IOException { + Workbook wb = _testDataProvider.createWorkbook(); + Sheet sheet = wb.createSheet(); + + { // create the Formula-Cell + Row row = sheet.createRow(0); + Cell cell = row.createCell(0); + cell.setCellFormula("A2"); + } + + { // check that it is there now + Row row = sheet.getRow(0); + + /* CTCell[] cArray = ((XSSFRow)row).getCTRow().getCArray(); + assertEquals(1, cArray.length);*/ + + Cell cell = row.getCell(0); + assertEquals(Cell.CELL_TYPE_FORMULA, cell.getCellType()); + } + + { // overwrite the row + Row row = sheet.createRow(0); + assertNotNull(row); + } + + { // creating a row in place of another should remove the existing data, + // check that the cell is gone now + Row row = sheet.getRow(0); + + /*CTCell[] cArray = ((XSSFRow)row).getCTRow().getCArray(); + assertEquals(0, cArray.length);*/ + + Cell cell = row.getCell(0); + assertNull(cell); + } + + // the calculation chain in XSSF is empty in a newly created workbook, so we cannot check if it is correctly updated + /*assertNull(((XSSFWorkbook)wb).getCalculationChain()); + assertNotNull(((XSSFWorkbook)wb).getCalculationChain().getCTCalcChain()); + assertNotNull(((XSSFWorkbook)wb).getCalculationChain().getCTCalcChain().getCArray()); + assertEquals(0, ((XSSFWorkbook)wb).getCalculationChain().getCTCalcChain().getCArray().length);*/ + + wb.close(); + } } diff --git a/test-data/spreadsheet/56574.xlsx b/test-data/spreadsheet/56574.xlsx new file mode 100644 index 0000000000..9cce54cc8f Binary files /dev/null and b/test-data/spreadsheet/56574.xlsx differ