Author: centic
Date: Sun May 18 19:18:27 2014
New Revision: 1595659
URL: http://svn.apache.org/r1595659
Log:
Bug 56170: Fix a problem with cells in workbooks becoming disconnected from
XMLBeans whenever columns need to be reordered during writing the file. This
happens because setCArray() disconnects any previously stored array-item but we
try to re-use them. So we need to recreate the CTCell and set it in the
XSSFCell to make this work in all currently tested cases.
Added:
poi/trunk/test-data/spreadsheet/56170.xlsx (with props)
Modified:
poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java
poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCell.java
Modified: poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java?rev=1595659&r1=1595658&r2=1595659&view=diff
==============================================================================
--- poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java
(original)
+++ poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java Sun
May 18 19:18:27 2014
@@ -70,7 +70,7 @@ public final class XSSFCell implements C
* the xml bean containing information about the cell's location, value,
* data type, formatting, and formula
*/
- private final CTCell _cell;
+ private CTCell _cell;
/**
* the XSSFRow this cell belongs to
@@ -940,6 +940,16 @@ public final class XSSFCell implements C
public CTCell getCTCell(){
return _cell;
}
+
+ /**
+ * 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.
+ */
+ @Internal
+ public void setCTCell(CTCell cell) {
+ _cell = cell;
+ }
/**
* Chooses a new boolean value for the cell when its type is changing.<p/>
Modified: poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java?rev=1595659&r1=1595658&r2=1595659&view=diff
==============================================================================
--- poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java
(original)
+++ poi/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java Sun May
18 19:18:27 2014
@@ -18,6 +18,7 @@
package org.apache.poi.xssf.usermodel;
import java.util.Iterator;
+import java.util.Map;
import java.util.TreeMap;
import org.apache.poi.ss.SpreadsheetVersion;
@@ -441,8 +442,9 @@ public class XSSFRow implements Row, Com
protected void onDocumentWrite(){
// check if cells in the CTRow are ordered
boolean isOrdered = true;
- if(_row.sizeOfCArray() != _cells.size()) isOrdered = false;
- else {
+ if(_row.sizeOfCArray() != _cells.size()) {
+ isOrdered = false;
+ } else {
int i = 0;
for (XSSFCell cell : _cells.values()) {
CTCell c1 = cell.getCTCell();
@@ -460,9 +462,18 @@ public class XSSFRow implements Row, Com
if(!isOrdered){
CTCell[] cArray = new CTCell[_cells.size()];
int i = 0;
- for (XSSFCell c : _cells.values()) {
- cArray[i++] = c.getCTCell();
+ for (Map.Entry<Integer, XSSFCell> entry : _cells.entrySet()) {
+ cArray[i] = (CTCell) entry.getValue().getCTCell().copy();
+
+ // we have to copy and re-create the XSSFCell here because the
+ // elements as otherwise setCArray below invalidates all the
columns!
+ // see Bug 56170, XMLBeans seems to always release previous
objects
+ // in the CArray, so we need to provide completely new ones
here!
+ //_cells.put(entry.getKey(), new XSSFCell(this, cArray[i]));
+ entry.getValue().setCTCell(cArray[i]);
+ i++;
}
+
_row.setCArray(cArray);
}
}
Modified:
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java?rev=1595659&r1=1595658&r2=1595659&view=diff
==============================================================================
---
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
(original)
+++
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
Sun May 18 19:18:27 2014
@@ -18,13 +18,7 @@
package org.apache.poi.xssf.usermodel;
import static org.hamcrest.core.IsEqual.equalTo;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
@@ -43,24 +37,7 @@ import org.apache.poi.ss.formula.Workboo
import org.apache.poi.ss.formula.eval.ErrorEval;
import org.apache.poi.ss.formula.eval.ValueEval;
import org.apache.poi.ss.formula.functions.Function;
-import org.apache.poi.ss.usermodel.BaseTestBugzillaIssues;
-import org.apache.poi.ss.usermodel.Cell;
-import org.apache.poi.ss.usermodel.CellStyle;
-import org.apache.poi.ss.usermodel.CellValue;
-import org.apache.poi.ss.usermodel.ClientAnchor;
-import org.apache.poi.ss.usermodel.Comment;
-import org.apache.poi.ss.usermodel.CreationHelper;
-import org.apache.poi.ss.usermodel.DataFormatter;
-import org.apache.poi.ss.usermodel.Drawing;
-import org.apache.poi.ss.usermodel.Font;
-import org.apache.poi.ss.usermodel.FormulaError;
-import org.apache.poi.ss.usermodel.FormulaEvaluator;
-import org.apache.poi.ss.usermodel.IndexedColors;
-import org.apache.poi.ss.usermodel.Name;
-import org.apache.poi.ss.usermodel.Row;
-import org.apache.poi.ss.usermodel.Sheet;
-import org.apache.poi.ss.usermodel.Workbook;
-import org.apache.poi.ss.usermodel.WorkbookFactory;
+import org.apache.poi.ss.usermodel.*;
import org.apache.poi.ss.util.AreaReference;
import org.apache.poi.ss.util.CellRangeAddress;
import org.apache.poi.ss.util.CellReference;
@@ -601,47 +578,77 @@ public final class TestXSSFBugs extends
}
/**
- * Various ways of removing a cell formula should all zap
- * the calcChain entry.
+ * Various ways of removing a cell formula should all zap the calcChain
+ * entry.
*/
@Test
public void bug49966() throws Exception {
- XSSFWorkbook wb =
XSSFTestDataSamples.openSampleWorkbook("shared_formulas.xlsx");
- XSSFSheet sheet = wb.getSheetAt(0);
-
- // CalcChain has lots of entries
- CalculationChain cc = wb.getCalculationChain();
- assertEquals("A2", cc.getCTCalcChain().getCArray(0).getR());
- assertEquals("A3", cc.getCTCalcChain().getCArray(1).getR());
- assertEquals("A4", cc.getCTCalcChain().getCArray(2).getR());
- assertEquals("A5", cc.getCTCalcChain().getCArray(3).getR());
- assertEquals("A6", cc.getCTCalcChain().getCArray(4).getR());
- assertEquals("A7", cc.getCTCalcChain().getCArray(5).getR());
- assertEquals("A8", cc.getCTCalcChain().getCArray(6).getR());
- assertEquals(40, cc.getCTCalcChain().sizeOfCArray());
-
- // Try various ways of changing the formulas
- // If it stays a formula, chain entry should remain
- // Otherwise should go
- sheet.getRow(1).getCell(0).setCellFormula("A1"); // stay
- sheet.getRow(2).getCell(0).setCellFormula(null); // go
- sheet.getRow(3).getCell(0).setCellType(Cell.CELL_TYPE_FORMULA); // stay
- sheet.getRow(4).getCell(0).setCellType(Cell.CELL_TYPE_STRING); // go
- sheet.getRow(5).removeCell(
- sheet.getRow(5).getCell(0) // go
- );
- sheet.getRow(6).getCell(0).setCellType(Cell.CELL_TYPE_BLANK); // go
- sheet.getRow(7).getCell(0).setCellValue((String)null); // go
+ XSSFWorkbook wb = XSSFTestDataSamples
+ .openSampleWorkbook("shared_formulas.xlsx");
+ XSSFSheet sheet = wb.getSheetAt(0);
+
+ Workbook wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+
+ // CalcChain has lots of entries
+ CalculationChain cc = wb.getCalculationChain();
+ assertEquals("A2", cc.getCTCalcChain().getCArray(0).getR());
+ assertEquals("A3", cc.getCTCalcChain().getCArray(1).getR());
+ assertEquals("A4", cc.getCTCalcChain().getCArray(2).getR());
+ assertEquals("A5", cc.getCTCalcChain().getCArray(3).getR());
+ assertEquals("A6", cc.getCTCalcChain().getCArray(4).getR());
+ assertEquals("A7", cc.getCTCalcChain().getCArray(5).getR());
+ assertEquals("A8", cc.getCTCalcChain().getCArray(6).getR());
+ assertEquals(40, cc.getCTCalcChain().sizeOfCArray());
+
+ wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+
+ // Try various ways of changing the formulas
+ // If it stays a formula, chain entry should remain
+ // Otherwise should go
+ sheet.getRow(1).getCell(0).setCellFormula("A1"); // stay
+ sheet.getRow(2).getCell(0).setCellFormula(null); // go
+ sheet.getRow(3).getCell(0).setCellType(Cell.CELL_TYPE_FORMULA); // stay
+ wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+ sheet.getRow(4).getCell(0).setCellType(Cell.CELL_TYPE_STRING); // go
+ wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+
+ validateCells(sheet);
+ sheet.getRow(5).removeCell(sheet.getRow(5).getCell(0)); // go
+ validateCells(sheet);
+ wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+
+ sheet.getRow(6).getCell(0).setCellType(Cell.CELL_TYPE_BLANK); // go
+ wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+ sheet.getRow(7).getCell(0).setCellValue((String) null); // go
- // Save and check
- wb = XSSFTestDataSamples.writeOutAndReadBack(wb);
- assertEquals(35, cc.getCTCalcChain().sizeOfCArray());
+ wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+
+ // Save and check
+ wb = XSSFTestDataSamples.writeOutAndReadBack(wb);
+ assertEquals(35, cc.getCTCalcChain().sizeOfCArray());
+
+ cc = wb.getCalculationChain();
+ assertEquals("A2", cc.getCTCalcChain().getCArray(0).getR());
+ assertEquals("A4", cc.getCTCalcChain().getCArray(1).getR());
+ assertEquals("A9", cc.getCTCalcChain().getCArray(2).getR());
+ }
- cc = wb.getCalculationChain();
- assertEquals("A2", cc.getCTCalcChain().getCArray(0).getR());
- assertEquals("A4", cc.getCTCalcChain().getCArray(1).getR());
- assertEquals("A9", cc.getCTCalcChain().getCArray(2).getR());
+ @Test
+ public void bug49966Row() throws Exception {
+ XSSFWorkbook wb = XSSFTestDataSamples
+ .openSampleWorkbook("shared_formulas.xlsx");
+ XSSFSheet sheet = wb.getSheetAt(0);
+ validateCells(sheet);
+ sheet.getRow(5).removeCell(sheet.getRow(5).getCell(0)); // go
+ validateCells(sheet);
+ }
+
+ private void validateCells(XSSFSheet sheet) {
+ for(Row row : sheet) {
+ // trigger handling
+ ((XSSFRow)row).onDocumentWrite();
+ }
}
@Test
Modified:
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCell.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCell.java?rev=1595659&r1=1595658&r2=1595659&view=diff
==============================================================================
---
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCell.java
(original)
+++
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCell.java
Sun May 18 19:18:27 2014
@@ -17,14 +17,21 @@
package org.apache.poi.xssf.usermodel;
-import org.apache.poi.ss.usermodel.*;
+import java.io.IOException;
+
+import org.apache.poi.ss.usermodel.BaseTestCell;
+import org.apache.poi.ss.usermodel.Cell;
+import org.apache.poi.ss.usermodel.DataFormatter;
+import org.apache.poi.ss.usermodel.Row;
+import org.apache.poi.ss.usermodel.Sheet;
+import org.apache.poi.ss.usermodel.Workbook;
+import org.apache.poi.ss.util.CellRangeAddress;
+import org.apache.poi.ss.util.CellReference;
import org.apache.poi.xssf.XSSFITestDataProvider;
+import org.apache.poi.xssf.XSSFTestDataSamples;
import org.apache.poi.xssf.model.SharedStringsTable;
-import org.openxmlformats.schemas.spreadsheetml.x2006.main.STCellType;
import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCell;
-
-import java.io.FileOutputStream;
-import java.io.IOException;
+import org.openxmlformats.schemas.spreadsheetml.x2006.main.STCellType;
/**
* @author Yegor Kozlov
@@ -276,5 +283,93 @@ public final class TestXSSFCell extends
}
}
}
-
+
+ public void test56170() throws IOException {
+ final Workbook wb =
XSSFTestDataSamples.openSampleWorkbook("56170.xlsx");
+ final XSSFSheet sheet = (XSSFSheet) wb.getSheetAt(0);
+
+ Workbook wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+ Cell cell;
+
+ // add some contents to table so that the table will need expansion
+ Row row = sheet.getRow(0);
+ wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+ cell = row.createCell(0);
+ wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+ cell.setCellValue("demo1");
+ wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+ cell = row.createCell(1);
+ wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+ cell.setCellValue("demo2");
+ wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+ cell = row.createCell(2);
+ wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+ cell.setCellValue("demo3");
+
+ wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+
+ row = sheet.getRow(1);
+ cell = row.createCell(0);
+ cell.setCellValue("demo1");
+ cell = row.createCell(1);
+ cell.setCellValue("demo2");
+ cell = row.createCell(2);
+ cell.setCellValue("demo3");
+
+ wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+
+ // expand table
+ XSSFTable table = sheet.getTables().get(0);
+ final CellReference startRef = table.getStartCellReference();
+ final CellReference endRef = table.getEndCellReference();
+ table.getCTTable().setRef(new CellRangeAddress(startRef.getRow(), 1,
startRef.getCol(), endRef.getCol()).formatAsString());
+
+ wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
+ assertNotNull(wbRead);
+
+ /*FileOutputStream stream = new
FileOutputStream("c:\\temp\\output.xlsx");
+ workbook.write(stream);
+ stream.close();*/
+ }
+
+ public void test56170Reproduce() throws IOException {
+ final Workbook wb = new XSSFWorkbook();
+ final Sheet sheet = wb.createSheet();
+ Row row = sheet.createRow(0);
+
+ // by creating Cells out of order we trigger the handling in
onDocumentWrite()
+ Cell cell1 = row.createCell(1);
+ Cell cell2 = row.createCell(0);
+
+ validateRow(row);
+
+ validateRow(row);
+
+ // once again with removing one cell
+ row.removeCell(cell1);
+
+ validateRow(row);
+
+ // once again with removing one cell
+ row.removeCell(cell1);
+
+ // now check again
+ validateRow(row);
+
+ // once again with removing one cell
+ row.removeCell(cell2);
+
+ // now check again
+ validateRow(row);
+ }
+
+ private void validateRow(Row row) {
+ // trigger bug with CArray handling
+ ((XSSFRow)row).onDocumentWrite();
+
+ for(Cell cell : row) {
+ cell.toString();
+ }
+ }
+
}
Added: poi/trunk/test-data/spreadsheet/56170.xlsx
URL:
http://svn.apache.org/viewvc/poi/trunk/test-data/spreadsheet/56170.xlsx?rev=1595659&view=auto
==============================================================================
Binary file - no diff available.
Propchange: poi/trunk/test-data/spreadsheet/56170.xlsx
------------------------------------------------------------------------------
svn:mime-type = application/octet-stream
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@poi.apache.org
For additional commands, e-mail: commits-h...@poi.apache.org
