From 2348794109399c8bf4070b2e03196bfcf3168191 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sat, 16 Jul 2016 20:52:39 +0000 Subject: [PATCH] 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 --- .../apache/poi/hssf/usermodel/HSSFSheet.java | 3 +- .../org/apache/poi/ss/usermodel/Sheet.java | 3 +- .../poi/ss/usermodel/helpers/RowShifter.java | 14 +++- .../apache/poi/xssf/streaming/SXSSFSheet.java | 3 +- .../apache/poi/xssf/usermodel/XSSFSheet.java | 3 +- .../ss/usermodel/BaseTestSheetShiftRows.java | 81 ++++++++++++++----- 6 files changed, 80 insertions(+), 27 deletions(-) diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java b/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java index b1cf0fceda..3ff7005ba4 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java +++ b/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

* * 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.

* * TODO Might want to add bounds checking here * diff --git a/src/java/org/apache/poi/ss/usermodel/Sheet.java b/src/java/org/apache/poi/ss/usermodel/Sheet.java index 3c064600b7..74ad8690e2 100644 --- a/src/java/org/apache/poi/ss/usermodel/Sheet.java +++ b/src/java/org/apache/poi/ss/usermodel/Sheet.java @@ -696,7 +696,8 @@ public interface Sheet extends Iterable { * *

* 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. *

* @param startRow the row to start shifting * @param endRow the row to end shifting 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 a168080004..680abff626 100644 --- a/src/java/org/apache/poi/ss/usermodel/helpers/RowShifter.java +++ b/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 shiftMergedRegions(int startRow, int endRow, int n) { List shiftedRegions = new ArrayList(); @@ -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); diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java index 336cd83e87..41e4e17f21 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java @@ -996,7 +996,8 @@ public class SXSSFSheet implements Sheet * *

* 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. *

* @param startRow the row to start shifting * @param endRow the row to end shifting diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java index c7cb28f5b9..44474b0b4c 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java @@ -2913,7 +2913,8 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet { * *

* 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. *

* @param startRow the row to start shifting * @param endRow the row to end shifting diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java index ac6037b42c..b11369b949 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java +++ b/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 + @Ignore("bug 56454: Incorrectly handles merged regions that do not contain column 0") @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);