Author: centic
Date: Sat Aug 19 16:31:45 2017
New Revision: 1805518

URL: http://svn.apache.org/viewvc?rev=1805518&view=rev
Log:
Fix 60384 and 60709: When shifting with merged regions we should correctly 
check if the region is moved along or needs to be removed because it is not 
fully kept via the shifting. This was broken by the fix for bug 59740, now 
additional unit tests ensure that it works better.

Added:
    poi/trunk/test-data/spreadsheet/60384.xlsx
    poi/trunk/test-data/spreadsheet/60709.xlsx
Modified:
    poi/trunk/src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java
    
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java
    
poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java

Modified: poi/trunk/src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java
URL: 
http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java?rev=1805518&r1=1805517&r2=1805518&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java 
(original)
+++ poi/trunk/src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java Sat 
Aug 19 16:31:45 2017
@@ -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
      */

Modified: 
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java
URL: 
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java?rev=1805518&r1=1805517&r2=1805518&view=diff
==============================================================================
--- 
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java
 (original)
+++ 
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java
 Sat Aug 19 16:31:45 2017
@@ -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.XSSFTestDataS
 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 TestXSSFSheetShiftRow
             skipTest(e);
         }
         
-
         workbook.close();
     }
 
@@ -486,4 +476,62 @@ public final class TestXSSFSheetShiftRow
         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();
+    }
 }

Modified: 
poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java
URL: 
http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java?rev=1805518&r1=1805517&r2=1805518&view=diff
==============================================================================
--- 
poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java 
(original)
+++ 
poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java 
Sat Aug 19 16:31:45 2017
@@ -502,8 +502,6 @@ public abstract class BaseTestSheetShift
      * 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 BaseTestSheetShift
     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 BaseTestSheetShift
         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 BaseTestSheetShift
         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);

Added: poi/trunk/test-data/spreadsheet/60384.xlsx
URL: 
http://svn.apache.org/viewvc/poi/trunk/test-data/spreadsheet/60384.xlsx?rev=1805518&view=auto
==============================================================================
Binary files poi/trunk/test-data/spreadsheet/60384.xlsx (added) and 
poi/trunk/test-data/spreadsheet/60384.xlsx Sat Aug 19 16:31:45 2017 differ

Added: poi/trunk/test-data/spreadsheet/60709.xlsx
URL: 
http://svn.apache.org/viewvc/poi/trunk/test-data/spreadsheet/60709.xlsx?rev=1805518&view=auto
==============================================================================
Binary files poi/trunk/test-data/spreadsheet/60709.xlsx (added) and 
poi/trunk/test-data/spreadsheet/60709.xlsx Sat Aug 19 16:31:45 2017 differ



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to