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 37ce7fba2..fe985ac71 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java @@ -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 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 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; } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java index aa2ae2280..a5050c92a 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java @@ -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 index 000000000..f273308ec Binary files /dev/null and b/test-data/spreadsheet/53798.xlsx differ