diff --git a/src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java b/src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java index 680abff626..13777dc909 100644 --- a/src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java +++ b/src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java @@ -27,8 +27,6 @@ import org.apache.poi.ss.usermodel.Row; import org.apache.poi.ss.usermodel.Sheet; import org.apache.poi.ss.util.CellRangeAddress; import org.apache.poi.util.Internal; -import org.apache.poi.util.POILogFactory; -import org.apache.poi.util.POILogger; /** * Helper for shifting rows up or down @@ -59,8 +57,9 @@ public abstract class RowShifter { for (int i = 0; i < size; i++) { CellRangeAddress merged = sheet.getMergedRegion(i); - // remove merged region that overlaps shifting - if (startRow + n <= merged.getFirstRow() && endRow + n >= merged.getLastRow()) { + // remove merged region that are replaced by the shifting, + // i.e. where the area includes something in the overwritten area + if(removalNeeded(merged, startRow, endRow, n)) { removedIndices.add(i); continue; } @@ -94,6 +93,24 @@ public abstract class RowShifter { return shiftedRegions; } + private boolean removalNeeded(CellRangeAddress merged, int startRow, int endRow, int n) { + final int movedRows = endRow - startRow + 1; + + // build a range of the rows that are overwritten, i.e. the target-area, but without + // rows that are moved along + final CellRangeAddress overwrite; + if(n > 0) { + // area is moved down => overwritten area is [endRow + n - movedRows, endRow + n] + overwrite = new CellRangeAddress(Math.max(endRow + 1, endRow + n - movedRows), endRow + n, 0, 0); + } else { + // area is moved up => overwritten area is [startRow + n, startRow + n + movedRows] + overwrite = new CellRangeAddress(startRow + n, Math.min(startRow - 1, startRow + n + movedRows), 0, 0); + } + + // if the merged-region and the overwritten area intersect, we need to remove it + return merged.intersects(overwrite); + } + /** * Updated named ranges */ 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 78e9d1bb25..07ba9f901a 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java @@ -17,22 +17,7 @@ package org.apache.poi.xssf.usermodel; -import static org.apache.poi.POITestCase.skipTest; -import static org.apache.poi.POITestCase.testPassesNow; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.fail; - -import java.io.IOException; - -import org.apache.poi.ss.usermodel.BaseTestSheetShiftRows; -import org.apache.poi.ss.usermodel.Cell; -import org.apache.poi.ss.usermodel.CellType; -import org.apache.poi.ss.usermodel.Comment; -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.*; import org.apache.poi.ss.util.CellAddress; import org.apache.poi.ss.util.CellUtil; import org.apache.poi.util.IOUtils; @@ -41,6 +26,12 @@ import org.apache.poi.xssf.XSSFTestDataSamples; import org.apache.xmlbeans.impl.values.XmlValueDisconnectedException; import org.junit.Test; +import java.io.IOException; + +import static org.apache.poi.POITestCase.skipTest; +import static org.apache.poi.POITestCase.testPassesNow; +import static org.junit.Assert.*; + public final class TestXSSFSheetShiftRows extends BaseTestSheetShiftRows { public TestXSSFSheetShiftRows(){ @@ -413,7 +404,6 @@ public final class TestXSSFSheetShiftRows extends BaseTestSheetShiftRows { skipTest(e); } - workbook.close(); } @@ -486,4 +476,62 @@ public final class TestXSSFSheetShiftRows extends BaseTestSheetShiftRows { sheet.shiftRows(1, 2, 3); IOUtils.closeQuietly(wb); } + + @Test + public void test60384() throws IOException { + XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("60384.xlsx"); + XSSFSheet sheet = wb.getSheetAt(0); + + assertEquals(2, sheet.getMergedRegions().size()); + assertEquals(7, sheet.getMergedRegion(0).getFirstRow()); + assertEquals(7, sheet.getMergedRegion(0).getLastRow()); + assertEquals(8, sheet.getMergedRegion(1).getFirstRow()); + assertEquals(8, sheet.getMergedRegion(1).getLastRow()); + + sheet.shiftRows(3, 8, 1); + + // after shifting, the two named regions should still be there as they + // are fully inside the shifted area + assertEquals(2, sheet.getMergedRegions().size()); + assertEquals(8, sheet.getMergedRegion(0).getFirstRow()); + assertEquals(8, sheet.getMergedRegion(0).getLastRow()); + assertEquals(9, sheet.getMergedRegion(1).getFirstRow()); + assertEquals(9, sheet.getMergedRegion(1).getLastRow()); + + /*OutputStream out = new FileOutputStream("/tmp/60384.xlsx"); + try { + wb.write(out); + } finally { + out.close(); + }*/ + + wb.close(); + } + + @Test + public void test60709() throws IOException { + XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("60709.xlsx"); + XSSFSheet sheet = wb.getSheetAt(0); + + assertEquals(1, sheet.getMergedRegions().size()); + assertEquals(2, sheet.getMergedRegion(0).getFirstRow()); + assertEquals(2, sheet.getMergedRegion(0).getLastRow()); + + sheet.shiftRows(1, sheet.getLastRowNum()+1, -1, true, false); + + // after shifting, the two named regions should still be there as they + // are fully inside the shifted area + assertEquals(1, sheet.getMergedRegions().size()); + assertEquals(1, sheet.getMergedRegion(0).getFirstRow()); + assertEquals(1, sheet.getMergedRegion(0).getLastRow()); + + /*OutputStream out = new FileOutputStream("/tmp/60709.xlsx"); + try { + wb.write(out); + } finally { + out.close(); + }*/ + + wb.close(); + } } diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java index cf910d4620..30764c4cb9 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java @@ -502,8 +502,6 @@ public abstract class BaseTestSheetShiftRows { * Unified test for: * bug 46742: XSSFSheet.shiftRows should shift hyperlinks * bug 52903: HSSFSheet.shiftRows should shift hyperlinks - * - * @throws IOException */ @Test public void testBug46742_52903_shiftHyperlinks() throws IOException { @@ -642,7 +640,7 @@ public abstract class BaseTestSheetShiftRows { public void shiftMergedRowsToMergedRowsUp() throws IOException { Workbook wb = _testDataProvider.createWorkbook(); Sheet sheet = wb.createSheet("test"); - populateSheetCells(sheet); + populateSheetCells(sheet, 2); CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4); @@ -661,9 +659,55 @@ public abstract class BaseTestSheetShiftRows { wb.close(); } - private void populateSheetCells(Sheet sheet) { + @Test + public void shiftMergedRowsToMergedRowsOverlappingMergedRegion() throws IOException { + Workbook wb = _testDataProvider.createWorkbook(); + Sheet sheet = wb.createSheet("test"); + populateSheetCells(sheet, 10); + + CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4); + CellRangeAddress A2_C2 = new CellRangeAddress(1, 7, 0, 2); + + sheet.addMergedRegion(A1_E1); + sheet.addMergedRegion(A2_C2); + + // A1:E1 should move to A5:E5 + // A2:C2 should be removed + sheet.shiftRows(0, 0, 4); + + assertEquals(1, sheet.getNumMergedRegions()); + assertEquals(CellRangeAddress.valueOf("A5:E5"), sheet.getMergedRegion(0)); + + wb.close(); + } + + @Test + public void bug60384ShiftMergedRegion() throws IOException { + Workbook wb = _testDataProvider.createWorkbook(); + Sheet sheet = wb.createSheet("test"); + populateSheetCells(sheet, 9); + + + CellRangeAddress A8_E8 = new CellRangeAddress(7, 7, 0, 4); + CellRangeAddress A9_C9 = new CellRangeAddress(8, 8, 0, 2); + + sheet.addMergedRegion(A8_E8); + sheet.addMergedRegion(A9_C9); + + // A1:E1 should be removed + // A2:C2 will be A1:C1 + sheet.shiftRows(3, sheet.getLastRowNum(), 1); + + assertEquals(2, sheet.getNumMergedRegions()); + assertEquals(CellRangeAddress.valueOf("A9:E9"), sheet.getMergedRegion(0)); + assertEquals(CellRangeAddress.valueOf("A10:C10"), sheet.getMergedRegion(1)); + + wb.close(); + } + + private void populateSheetCells(Sheet sheet, int rowCount) { // populate sheet cells - for (int i = 0; i < 2; i++) { + for (int i = 0; i < rowCount; i++) { Row row = sheet.createRow(i); for (int j = 0; j < 5; j++) { Cell cell = row.createCell(j); @@ -678,7 +722,7 @@ public abstract class BaseTestSheetShiftRows { Sheet sheet = wb.createSheet("test"); // populate sheet cells - populateSheetCells(sheet); + populateSheetCells(sheet, 2); CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4); CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2); diff --git a/test-data/spreadsheet/60384.xlsx b/test-data/spreadsheet/60384.xlsx new file mode 100644 index 0000000000..3695a439ba Binary files /dev/null and b/test-data/spreadsheet/60384.xlsx differ diff --git a/test-data/spreadsheet/60709.xlsx b/test-data/spreadsheet/60709.xlsx new file mode 100644 index 0000000000..3aa58777bb Binary files /dev/null and b/test-data/spreadsheet/60709.xlsx differ