]> source.dussan.org Git - poi.git/commitdiff
Fix 60384 and 60709: When shifting with merged regions we should correctly check...
authorDominik Stadler <centic@apache.org>
Sat, 19 Aug 2017 16:31:45 +0000 (16:31 +0000)
committerDominik Stadler <centic@apache.org>
Sat, 19 Aug 2017 16:31:45 +0000 (16:31 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1805518 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java
src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java
src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java
test-data/spreadsheet/60384.xlsx [new file with mode: 0644]
test-data/spreadsheet/60709.xlsx [new file with mode: 0644]

index 680abff626b82336b823f8286bf3d1f59140bef9..13777dc909d4032c005a2bbfaba2c0401506d401 100644 (file)
@@ -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
      */
index 78e9d1bb254d10a1750ad4ccfa6e8018dcfce70e..07ba9f901ab3fd68a010d3e81260d4037c204926 100644 (file)
 
 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();
+    }
 }
index cf910d4620ca371aa142630280b87505475916c1..30764c4cb9efbfd653c3d964aef5efdc92c30359 100644 (file)
@@ -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 (file)
index 0000000..3695a43
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 (file)
index 0000000..3aa5877
Binary files /dev/null and b/test-data/spreadsheet/60709.xlsx differ