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

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<Row> it = rowIterator() ; it.hasNext() ; ) {
             XSSFRow row = (XSSFRow)it.next();

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<String, Object[]> data;
+        data = new TreeMap<String, Object[]>();
+        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<String> 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();
+    }    
 }

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();
+    }
 }