optimized serialization of XSSFRow - avoid re-ordering of CTCell beans if they are already ordered

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@949375 13f79535-47bb-0310-9956-ffa450edef68

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<XSSFRow> {
 
     /**
      * Fired when the document is written to an output stream.
-     * <p>
+     *
-     * Attaches CTCell beans to the underlying CTRow bean
+     * @see org.apache.poi.xssf.usermodel.XSSFSheet#write(java.io.OutputStream) ()
-     * </p>
-     * @see org.apache.poi.xssf.usermodel.XSSFSheet#commit()
      */
     protected void onDocumentWrite(){
-        ArrayList<CTCell> cArray = new ArrayList<CTCell>(_cells.size());
+        // check if cells in the CTRow are ordered
-        //create array of CTCell objects.
+        boolean isOrdered = true;
-        //TreeMap's value iterator ensures that the cells are ordered by columnIndex in the ascending order
+        if(_row.sizeOfCArray() != _cells.size()) isOrdered = false;
-        for (Cell cell : _cells.values()) {
+        else {
-            XSSFCell c = (XSSFCell)cell;
+            int i = 0;
-            cArray.add(c.getCTCell());
+            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()]));
     }
 
     /**

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()];
+        boolean isOrdered = true;
+        if(sheetData.sizeOfRowArray() != _rows.size()) isOrdered = false;
+        else {
             int i = 0;
-        for(CTRow ctrow : sheetData.getRowArray()){
+            CTRow[] xrow = 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();
+                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);
         }
     }
 

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.*;
-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;
 
 
 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());
+        CTSheetData sheetData = wsh.getSheetData();
-        XSSFRow row1 = sheet.createRow(1);
+        assertEquals(0, sheetData.sizeOfRowArray());
-        row1.createCell(1);
+
+        XSSFRow row1 = sheet.createRow(2);
         row1.createCell(2);
-        assertEquals(1, wsh.getSheetData().sizeOfRowArray());
+        row1.createCell(1);
-        assertEquals(2, wsh.getSheetData().getRowArray(0).sizeOfCArray());
+
+        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);
+        row2 = sheet.createRow(1);
-        assertEquals(1, wsh.getSheetData().sizeOfRowArray());
+        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());
+
     }
 }