diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java index 61535c5cd..f43a05fad 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRow.java @@ -18,6 +18,7 @@ package org.apache.poi.xssf.usermodel; import java.util.ArrayList; +import java.util.Arrays; import java.util.Iterator; import java.util.TreeMap; @@ -384,20 +385,37 @@ public class XSSFRow implements Row, Comparable { /** * Fired when the document is written to an output stream. - *

- * Attaches CTCell beans to the underlying CTRow bean - *

- * @see org.apache.poi.xssf.usermodel.XSSFSheet#commit() + * + * @see org.apache.poi.xssf.usermodel.XSSFSheet#write(java.io.OutputStream) () */ protected void onDocumentWrite(){ - ArrayList cArray = new ArrayList(_cells.size()); - //create array of CTCell objects. - //TreeMap's value iterator ensures that the cells are ordered by columnIndex in the ascending order - for (Cell cell : _cells.values()) { - XSSFCell c = (XSSFCell)cell; - cArray.add(c.getCTCell()); + // check if cells in the CTRow are ordered + boolean isOrdered = true; + if(_row.sizeOfCArray() != _cells.size()) isOrdered = false; + else { + int i = 0; + CTCell[] xcell = _row.getCArray(); + for (XSSFCell cell : _cells.values()) { + CTCell c1 = cell.getCTCell(); + CTCell c2 = xcell[i++]; + + String r1 = c1.getR(); + String r2 = c2.getR(); + if (!(r1==null ? r2==null : r1.equals(r2))){ + isOrdered = false; + break; + } + } + } + + if(!isOrdered){ + CTCell[] cArray = new CTCell[_cells.size()]; + int i = 0; + for (XSSFCell c : _cells.values()) { + cArray[i++] = c.getCTCell(); + } + _row.setCArray(cArray); } - _row.setCArray(cArray.toArray(new CTCell[cArray.size()])); } /** 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 460055369..34db7c403 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java @@ -2464,20 +2464,28 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet { // rows in the internal model (_rows) are always ordered while // CTRow beans held by CTSheetData may be not, for example, user can // insert rows in random order, shift rows after insertion, etc. - Integer [] curRows = new Integer[sheetData.sizeOfRowArray()]; - int i = 0; - for(CTRow ctrow : sheetData.getRowArray()){ - curRows[i++] = (int)(ctrow.getR() - 1); - } - Integer [] ordRows = _rows.keySet().toArray(new Integer[_rows.size()]); - if(!Arrays.equals(curRows, ordRows)){ - // The order of rows in CTSheetData and internal model does not match - CTRow[] orderedCTRows = new CTRow[_rows.size()]; - i = 0; - for(XSSFRow row : _rows.values()){ - orderedCTRows[i++] = row.getCTRow(); + boolean isOrdered = true; + if(sheetData.sizeOfRowArray() != _rows.size()) isOrdered = false; + else { + int i = 0; + CTRow[] xrow = sheetData.getRowArray(); + for (XSSFRow row : _rows.values()) { + CTRow c1 = row.getCTRow(); + CTRow c2 = xrow[i++]; + if (c1.getR() != c2.getR()){ + isOrdered = false; + break; + } } - sheetData.setRowArray(orderedCTRows); + } + + if(!isOrdered){ + CTRow[] cArray = new CTRow[_rows.size()]; + int i = 0; + for(XSSFRow row : _rows.values()){ + cArray[i++] = row.getCTRow(); + } + sheetData.setRowArray(cArray); } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java index c1302812e..db4a26c49 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java @@ -24,13 +24,7 @@ import org.apache.poi.xssf.XSSFTestDataSamples; import org.apache.poi.xssf.model.CommentsTable; import org.apache.poi.xssf.model.StylesTable; import org.apache.poi.xssf.usermodel.helpers.ColumnHelper; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCol; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCols; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTComments; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTRow; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTWorksheet; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTXf; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.STPane; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.*; public final class TestXSSFSheet extends BaseTestSheet { @@ -894,21 +888,84 @@ public final class TestXSSFSheet extends BaseTestSheet { assertSame(comment1, sheet1.getCommentsTable(true)); } + /** + * Rows and cells can be created in random order, + * but serialization forces strict ascending order of the CTRow and CTCell xml beans + */ public void testCreateRow() { XSSFWorkbook workbook = new XSSFWorkbook(); XSSFSheet sheet = workbook.createSheet(); CTWorksheet wsh = sheet.getCTWorksheet(); - assertEquals(0, wsh.getSheetData().sizeOfRowArray()); - XSSFRow row1 = sheet.createRow(1); - row1.createCell(1); + CTSheetData sheetData = wsh.getSheetData(); + assertEquals(0, sheetData.sizeOfRowArray()); + + XSSFRow row1 = sheet.createRow(2); row1.createCell(2); - assertEquals(1, wsh.getSheetData().sizeOfRowArray()); - assertEquals(2, wsh.getSheetData().getRowArray(0).sizeOfCArray()); + row1.createCell(1); + + XSSFRow row2 = sheet.createRow(1); + row2.createCell(2); + row2.createCell(1); + row2.createCell(0); + + XSSFRow row3 = sheet.createRow(0); + row3.createCell(3); + row3.createCell(0); + row3.createCell(2); + row3.createCell(5); + + + CTRow[] xrow = sheetData.getRowArray(); + assertEquals(3, xrow.length); + + //rows are unsorted: {2, 1, 0} + assertEquals(2, xrow[0].sizeOfCArray()); + assertEquals(3, xrow[0].getR()); + assertTrue(xrow[0].equals(row1.getCTRow())); + + assertEquals(3, xrow[1].sizeOfCArray()); + assertEquals(2, xrow[1].getR()); + assertTrue(xrow[1].equals(row2.getCTRow())); + + assertEquals(4, xrow[2].sizeOfCArray()); + assertEquals(1, xrow[2].getR()); + assertTrue(xrow[2].equals(row3.getCTRow())); + + CTCell[] xcell = xrow[2].getCArray(); + assertEquals("D1", xcell[0].getR()); + assertEquals("A1", xcell[1].getR()); + assertEquals("C1", xcell[2].getR()); + assertEquals("F1", xcell[3].getR()); //re-creating a row does NOT add extra data to the parent - sheet.createRow(1); - assertEquals(1, wsh.getSheetData().sizeOfRowArray()); + row2 = sheet.createRow(1); + assertEquals(3, sheetData.sizeOfRowArray()); //existing cells are invalidated - assertEquals(0, wsh.getSheetData().getRowArray(0).sizeOfCArray()); + assertEquals(0, sheetData.getRowArray(1).sizeOfCArray()); + assertEquals(0, row2.getPhysicalNumberOfCells()); + + workbook = XSSFTestDataSamples.writeOutAndReadBack(workbook); + sheet = workbook.getSheetAt(0); + wsh = sheet.getCTWorksheet(); + xrow = sheetData.getRowArray(); + assertEquals(3, xrow.length); + + //rows are sorted: {0, 1, 2} + assertEquals(4, xrow[0].sizeOfCArray()); + assertEquals(1, xrow[0].getR()); + //cells are now sorted + xcell = xrow[0].getCArray(); + assertEquals("A1", xcell[0].getR()); + assertEquals("C1", xcell[1].getR()); + assertEquals("D1", xcell[2].getR()); + assertEquals("F1", xcell[3].getR()); + + + assertEquals(0, xrow[1].sizeOfCArray()); + assertEquals(2, xrow[1].getR()); + + assertEquals(2, xrow[2].sizeOfCArray()); + assertEquals(3, xrow[2].getR()); + } }