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.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1595659 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Dominik Stadler 2014-05-18 19:18:27 +00:00
parent 764ec1df17
commit ecf9194b65
5 changed files with 194 additions and 71 deletions

View File

@ -70,7 +70,7 @@ public final class XSSFCell implements Cell {
* 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
@ -941,6 +941,16 @@ public final class XSSFCell implements Cell {
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/>
*

View File

@ -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, Comparable<XSSFRow> {
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, Comparable<XSSFRow> {
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);
}
}

View File

@ -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.WorkbookEvaluator;
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 BaseTestBugzillaIssues {
}
/**
* 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);
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());
Workbook 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
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
// 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());
// Save and check
wb = XSSFTestDataSamples.writeOutAndReadBack(wb);
assertEquals(35, cc.getCTCalcChain().sizeOfCArray());
wbRead = XSSFTestDataSamples.writeOutAndReadBack(wb);
cc = wb.getCalculationChain();
assertEquals("A2", cc.getCTCalcChain().getCArray(0).getR());
assertEquals("A4", cc.getCTCalcChain().getCArray(1).getR());
assertEquals("A9", cc.getCTCalcChain().getCArray(2).getR());
// 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
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());
}
@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

View File

@ -17,15 +17,22 @@
package org.apache.poi.xssf.usermodel;
import org.apache.poi.ss.usermodel.*;
import org.apache.poi.xssf.XSSFITestDataProvider;
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.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.CTCell;
import org.openxmlformats.schemas.spreadsheetml.x2006.main.STCellType;
/**
* @author Yegor Kozlov
*/
@ -277,4 +284,92 @@ public final class TestXSSFCell extends BaseTestCell {
}
}
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();
}
}
}

Binary file not shown.