You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by ce...@apache.org on 2017/08/19 16:31:45 UTC
svn commit: r1805518 - in /poi/trunk:
src/java/org/apache/poi/ss/usermodel/helpers/
src/ooxml/testcases/org/apache/poi/xssf/usermodel/
src/testcases/org/apache/poi/ss/usermodel/ test-data/spreadsheet/
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: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org