Apply patch to fix bug 59740: Sheet.shiftRows incorrectly shifts merged region on exists merged region

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

src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java

@@ -1509,7 +1509,8 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
      * Code ensures that rows don't wrap around<p>
      * 
      * Additionally shifts merged regions that are completely defined in these
-     * rows (ie. merged 2 cells on a row to be shifted).<p>
+     * rows (ie. merged 2 cells on a row to be shifted). All merged regions that are
+     * completely overlaid by shifting will be deleted.<p>
      * 
      * TODO Might want to add bounds checking here
      *

src/java/org/apache/poi/ss/usermodel/Sheet.java

@@ -696,7 +696,8 @@ public interface Sheet extends Iterable<Row> {
      *
      * <p>
      * Additionally shifts merged regions that are completely defined in these
-     * rows (ie. merged 2 cells on a row to be shifted).
+     * rows (ie. merged 2 cells on a row to be shifted). All merged regions that are
+     * completely overlaid by shifting will be deleted.
      * <p>
      * @param startRow the row to start shifting
      * @param endRow the row to end shifting

src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java

@@ -36,7 +36,6 @@ import org.apache.poi.util.POILogger;
  * This abstract class exists to consolidate duplicated code between XSSFRowShifter and HSSFRowShifter (currently methods sprinkled throughout HSSFSheet)
  */
 public abstract class RowShifter {
-    private static final POILogger logger = POILogFactory.getLogger(RowShifter.class);
     protected final Sheet sheet;
 
     public RowShifter(Sheet sh) {
@@ -44,12 +43,13 @@ public abstract class RowShifter {
     }
 
     /**
-     * Shifts, grows, or shrinks the merged regions due to a row shift
+     * Shifts, grows, or shrinks the merged regions due to a row shift.
+     * Merged regions that are completely overlaid by shifting will be deleted.
      *
      * @param startRow the row to start shifting
      * @param endRow   the row to end shifting
      * @param n        the number of rows to shift
-     * @return an array of affected merged regions
+     * @return an array of affected merged regions, doesn't contain deleted ones
      */
     public List<CellRangeAddress> shiftMergedRegions(int startRow, int endRow, int n) {
         List<CellRangeAddress> shiftedRegions = new ArrayList<CellRangeAddress>();
@@ -59,6 +59,12 @@ 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()) {
+                removedIndices.add(i);
+                continue;
+            }
+
             boolean inStart = (merged.getFirstRow() >= startRow || merged.getLastRow() >= startRow);
             boolean inEnd = (merged.getFirstRow() <= endRow || merged.getLastRow() <= endRow);
 
@@ -114,7 +120,7 @@ public abstract class RowShifter {
      * is of type LINK_DOCUMENT and refers to a cell that was shifted). Hyperlinks
      * do not track the content they point to.
      *
-     * @param shifter
+     * @param shifter the formula shifting policy
      */
     public abstract void updateHyperlinks(FormulaShifter shifter);
 

src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java

@@ -996,7 +996,8 @@ public class SXSSFSheet implements Sheet
      *
      * <p>
      * Additionally shifts merged regions that are completely defined in these
-     * rows (ie. merged 2 cells on a row to be shifted).
+     * rows (ie. merged 2 cells on a row to be shifted). All merged regions that are
+     * completely overlaid by shifting will be deleted.
      * <p>
      * @param startRow the row to start shifting
      * @param endRow the row to end shifting

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java

@@ -2913,7 +2913,8 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
      *
      * <p>
      * Additionally shifts merged regions that are completely defined in these
-     * rows (ie. merged 2 cells on a row to be shifted).
+     * rows (ie. merged 2 cells on a row to be shifted). All merged regions that are
+     * completely overlaid by shifting will be deleted.
      * <p>
      * @param startRow the row to start shifting
      * @param endRow the row to end shifting

src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java

@@ -130,20 +130,6 @@ public abstract class BaseTestSheetShiftRows {
         wb.close();
     }
 
-    /**
-     * Tests when shifting the first row.
-     */
-    @Test
-    public final void testActiveCell() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet s = wb.createSheet();
-
-        s.createRow(0).createCell(0).setCellValue("TEST1");
-        s.createRow(3).createCell(0).setCellValue("TEST2");
-        s.shiftRows(0,4,1);
-        wb.close();
-    }
-
     /**
      * When shifting rows, the page breaks should go with it
      */
@@ -307,8 +293,8 @@ public abstract class BaseTestSheetShiftRows {
         assertEquals("A3:C3", region.formatAsString());
         wb.close();
     }
-    
+
-    @Ignore
+    @Ignore("bug 56454: Incorrectly handles merged regions that do not contain column 0")
     @Test
     public final void shiftWithMergedRegions_bug56454() throws IOException {
         Workbook wb = _testDataProvider.createWorkbook();
@@ -602,8 +588,7 @@ public abstract class BaseTestSheetShiftRows {
         read.close();
     }
     
-    // bug 56454: Incorrectly handles merged regions that do not contain column 0
+    @Ignore("bug 56454: Incorrectly handles merged regions that do not contain column 0")
-    @Ignore
     @Test
     public void shiftRowsWithMergedRegionsThatDoNotContainColumnZero() throws IOException {
         Workbook wb = _testDataProvider.createWorkbook();
@@ -634,7 +619,65 @@ public abstract class BaseTestSheetShiftRows {
         
         wb.close();
     }
-    
+
+    @Test
+    public void shiftMergedRowsToMergedRowsUp() throws IOException {
+        Workbook wb = _testDataProvider.createWorkbook();
+        Sheet sheet = wb.createSheet("test");
+        populateSheetCells(sheet);
+
+
+        CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
+        CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2);
+
+        sheet.addMergedRegion(A1_E1);
+        sheet.addMergedRegion(A2_C2);
+
+        // A1:E1 should be removed
+        // A2:C2 will be A1:C1
+        sheet.shiftRows(1, sheet.getLastRowNum(), -1);
+
+        assertEquals(1, sheet.getNumMergedRegions());
+        assertEquals(CellRangeAddress.valueOf("A1:C1"), sheet.getMergedRegion(0));
+
+        wb.close();
+    }
+
+    private void populateSheetCells(Sheet sheet) {
+        // populate sheet cells
+        for (int i = 0; i < 2; i++) {
+            Row row = sheet.createRow(i);
+            for (int j = 0; j < 5; j++) {
+                Cell cell = row.createCell(j);
+                cell.setCellValue(i + "x" + j);
+            }
+        }
+    }
+
+    @Test
+    public void shiftMergedRowsToMergedRowsDown() throws IOException {
+        Workbook wb = _testDataProvider.createWorkbook();
+        Sheet sheet = wb.createSheet("test");
+
+        // populate sheet cells
+        populateSheetCells(sheet);
+
+        CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
+        CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2);
+
+        sheet.addMergedRegion(A1_E1);
+        sheet.addMergedRegion(A2_C2);
+
+        // A1:E1 should be moved to A2:E2
+        // A2:C2 will be removed
+        sheet.shiftRows(0, 0, 1);
+
+        assertEquals(1, sheet.getNumMergedRegions());
+        assertEquals(CellRangeAddress.valueOf("A2:E2"), sheet.getMergedRegion(0));
+
+        wb.close();
+    }
+
     private void createHyperlink(CreationHelper helper, Cell cell, int linkType, String ref) {
         cell.setCellValue(ref);
         Hyperlink link = helper.createHyperlink(linkType);