]> source.dussan.org Git - poi.git/commitdiff
Bug 53798: Add fix for XmlValueDisconnectException during shifting rows
authorDominik Stadler <centic@apache.org>
Mon, 9 Sep 2013 09:24:05 +0000 (09:24 +0000)
committerDominik Stadler <centic@apache.org>
Mon, 9 Sep 2013 09:24:05 +0000 (09:24 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1521012 13f79535-47bb-0310-9956-ffa450edef68

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java
test-data/spreadsheet/53798.xlsx [new file with mode: 0644]

index 37ce7fba2cb100838d97efaefbd61c1e6a4ce3b3..fe985ac71d20c2bf2a25ffa435cbe2d04a3e575f 100644 (file)
@@ -2341,26 +2341,26 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
      */
     @SuppressWarnings("deprecation") //YK: getXYZArray() array accessors are deprecated in xmlbeans with JDK 1.5 support
     public void shiftRows(int startRow, int endRow, int n, boolean copyRowHeight, boolean resetOriginalRowHeight) {
+       // first remove all rows which will be overwritten
        for (Iterator<Row> it = rowIterator() ; it.hasNext() ; ) {
             XSSFRow row = (XSSFRow)it.next();
             int rownum = row.getRowNum();
-            if(rownum < startRow) continue;
-
-            if (!copyRowHeight) {
-                row.setHeight((short)-1);
-            }
-
+            
+            // check if we should remove this row as it will be overwritten by the data later
             if (removeRow(startRow, endRow, n, rownum)) {
                // remove row from worksheet.getSheetData row array
                int idx = _rows.headMap(row.getRowNum()).size();
-                worksheet.getSheetData().removeRow(idx);
+               worksheet.getSheetData().removeRow(idx);
                 // remove row from _rows
                 it.remove();
             }
-            else if (rownum >= startRow && rownum <= endRow) {
-                row.shift(n);
-            }
+       }
 
+       // then do the actual moving and also adjust comments/rowHeight 
+       for (Iterator<Row> it = rowIterator() ; it.hasNext() ; ) {
+            XSSFRow row = (XSSFRow)it.next();
+            int rownum = row.getRowNum();
+            
             if(sheetComments != null){
                 //TODO shift Note's anchor in the associated /xl/drawing/vmlDrawings#.vml
                 CTCommentList lst = sheetComments.getCTComments().getCommentList();
@@ -2372,6 +2372,14 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
                     }
                 }
             }
+            
+            if(rownum < startRow || rownum > endRow) continue;
+
+            if (!copyRowHeight) {
+                row.setHeight((short)-1);
+            }
+
+            row.shift(n);
         }
         XSSFRowShifter rowShifter = new XSSFRowShifter(this);
 
@@ -2625,7 +2633,9 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
     }
 
     private boolean removeRow(int startRow, int endRow, int n, int rownum) {
+       // is this row in the target-window where the moved rows will land?
         if (rownum >= (startRow + n) && rownum <= (endRow + n)) {
+               // only remove it if the current row is not part of the data that is copied
             if (n > 0 && rownum > endRow) {
                 return true;
             }
index aa2ae228095447bb7dcef79ec2318d5a1668ba0c..a5050c92ac42dbbfbe5fbf1f9ae1878a6046eff8 100644 (file)
@@ -21,6 +21,9 @@ import java.io.IOException;
 
 import org.apache.poi.ss.usermodel.BaseTestSheetShiftRows;
 import org.apache.poi.ss.usermodel.Cell;
+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.CellUtil;
 import org.apache.poi.xssf.XSSFITestDataProvider;
 import org.apache.poi.xssf.XSSFTestDataSamples;
@@ -34,11 +37,13 @@ public final class TestXSSFSheetShiftRows extends BaseTestSheetShiftRows {
         super(XSSFITestDataProvider.instance);
     }
 
-    public void testShiftRowBreaks() { // disabled test from superclass
+    @Override
+       public void testShiftRowBreaks() { // disabled test from superclass
         // TODO - support shifting of page breaks
     }
 
-    public void testShiftWithComments() { // disabled test from superclass
+    @Override
+       public void testShiftWithComments() { // disabled test from superclass
         // TODO - support shifting of comments.
     }
 
@@ -54,4 +59,86 @@ public final class TestXSSFSheetShiftRows extends BaseTestSheetShiftRows {
                cell = CellUtil.getCell(sheet.getRow(3), 0);
                assertEquals("X", cell.getStringCellValue());
        }
+       
+
+       public void testBug53798() throws IOException {
+               // NOTE that for HSSF (.xls) negative shifts combined with positive ones do work as expected  
+               Workbook wb = XSSFTestDataSamples.openSampleWorkbook("53798.xlsx");
+
+               Sheet testSheet = wb.getSheetAt(0);
+               // 1) corrupted xlsx (unreadable data in the first row of a shifted group) already comes about  
+               // when shifted by less than -1 negative amount (try -2)
+               testSheet.shiftRows(3, 3, -2);
+               
+               Row newRow = null; Cell newCell = null;
+               // 2) attempt to create a new row IN PLACE of a removed row by a negative shift causes corrupted 
+               // xlsx file with  unreadable data in the negative shifted row. 
+               // NOTE it's ok to create any other row.
+               newRow = testSheet.createRow(3);
+               newCell = newRow.createCell(0);
+               newCell.setCellValue("new Cell in row "+newRow.getRowNum());
+               
+               // 3) once a negative shift has been made any attempt to shift another group of rows 
+               // (note: outside of previously negative shifted rows) by a POSITIVE amount causes POI exception: 
+               // org.apache.xmlbeans.impl.values.XmlValueDisconnectedException.
+               // NOTE: another negative shift on another group of rows is successful, provided no new rows in  
+               // place of previously shifted rows were attempted to be created as explained above.
+               testSheet.shiftRows(6, 7, 1);   // -- CHANGE the shift to positive once the behaviour of  
+                                                                               // the above has been tested
+               
+               //saveReport(wb, new File("/tmp/53798.xlsx"));
+               Workbook read = XSSFTestDataSamples.writeOutAndReadBack(wb);
+               assertNotNull(read);
+               
+               Sheet readSheet = read.getSheetAt(0);
+               verifyCellContent(readSheet, 0, "0.0");
+               verifyCellContent(readSheet, 1, "3.0");
+               verifyCellContent(readSheet, 2, "2.0");
+               verifyCellContent(readSheet, 3, "new Cell in row 3");
+               verifyCellContent(readSheet, 4, "4.0");
+               verifyCellContent(readSheet, 5, "5.0");
+               verifyCellContent(readSheet, 6, null);
+               verifyCellContent(readSheet, 7, "6.0");
+               verifyCellContent(readSheet, 8, "7.0");
+       }
+
+       private void verifyCellContent(Sheet readSheet, int row, String expect) {
+               Row readRow = readSheet.getRow(row);
+               if(expect == null) {
+                       assertNull(readRow);
+                       return;
+               }
+               Cell readCell = readRow.getCell(0);
+               if(readCell.getCellType() == Cell.CELL_TYPE_NUMERIC) {
+                       assertEquals(expect, Double.toString(readCell.getNumericCellValue()));
+               } else {
+                       assertEquals(expect, readCell.getStringCellValue());
+               }
+       }
+       
+       public void testBug53798a() throws IOException {
+               Workbook wb = XSSFTestDataSamples.openSampleWorkbook("53798.xlsx");
+
+               Sheet testSheet = wb.getSheetAt(0);
+               testSheet.shiftRows(3, 3, -1);
+        for (Row r : testSheet) {
+               r.getRowNum();
+        }
+               testSheet.shiftRows(6, 6, 1);
+               
+               //saveReport(wb, new File("/tmp/53798.xlsx"));
+               Workbook read = XSSFTestDataSamples.writeOutAndReadBack(wb);
+               assertNotNull(read);
+               
+               Sheet readSheet = read.getSheetAt(0);
+               verifyCellContent(readSheet, 0, "0.0");
+               verifyCellContent(readSheet, 1, "1.0");
+               verifyCellContent(readSheet, 2, "3.0");
+               verifyCellContent(readSheet, 3, null);
+               verifyCellContent(readSheet, 4, "4.0");
+               verifyCellContent(readSheet, 5, "5.0");
+               verifyCellContent(readSheet, 6, null);
+               verifyCellContent(readSheet, 7, "6.0");
+               verifyCellContent(readSheet, 8, "8.0");
+       }
 }
diff --git a/test-data/spreadsheet/53798.xlsx b/test-data/spreadsheet/53798.xlsx
new file mode 100644 (file)
index 0000000..f273308
Binary files /dev/null and b/test-data/spreadsheet/53798.xlsx differ