From 6c1b0f30b079d9ce89b80a41f70d8ee01b83617e Mon Sep 17 00:00:00 2001 From: Javen O'Neal Date: Sun, 29 Nov 2015 14:45:24 +0000 Subject: [PATCH] Patch from Lyle for bug 52903: HSSFSheet.shiftRows does not shift hyperlink references git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1717069 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/hssf/usermodel/HSSFSheet.java | 55 +++++++- .../usermodel/TestXSSFSheetShiftRows.java | 107 --------------- .../ss/usermodel/BaseTestSheetShiftRows.java | 122 ++++++++++++++++++ 3 files changed, 175 insertions(+), 109 deletions(-) diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java b/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java index 862f08ce96..7f67794f14 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java @@ -1376,6 +1376,7 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet { * @param endRow the row to end shifting * @param n the number of rows to shift */ + @Override public void shiftRows(int startRow, int endRow, int n) { shiftRows(startRow, endRow, n, false, false); } @@ -1397,6 +1398,7 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet { * @param copyRowHeight whether to copy the row height during the shift * @param resetOriginalRowHeight whether to set the original row's height to the default */ + @Override public void shiftRows(int startRow, int endRow, int n, boolean copyRowHeight, boolean resetOriginalRowHeight) { shiftRows(startRow, endRow, n, copyRowHeight, resetOriginalRowHeight, true); } @@ -1422,6 +1424,9 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet { public void shiftRows(int startRow, int endRow, int n, boolean copyRowHeight, boolean resetOriginalRowHeight, boolean moveComments) { int s, inc; + if (endRow < startRow) { + throw new IllegalArgumentException("startRow must be less than or equal to endRow. To shift rows up, use n<0."); + } if (n < 0) { s = startRow; inc = 1; @@ -1433,12 +1438,29 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet { return; } + // Shift comments if (moveComments) { _sheet.getNoteRecords(); } + // Shift Merged Regions shiftMerged(startRow, endRow, n, true); + + // Shift Row Breaks _sheet.getPageSettings().shiftRowBreaks(startRow, endRow, n); + + // Delete overwritten hyperlinks + final int firstOverwrittenRow = startRow + n; + final int lastOverwrittenRow = endRow + n; + for (HSSFHyperlink link : getHyperlinkList()) { + // If hyperlink is fully contained in the rows that will be overwritten, delete the hyperlink + if (firstOverwrittenRow <= link.getFirstRow() && + link.getFirstRow() <= lastOverwrittenRow && + lastOverwrittenRow <= link.getLastRow() && + link.getLastRow() <= lastOverwrittenRow) { + removeHyperlink(link); + } + } for (int rowNum = s; rowNum >= startRow && rowNum <= endRow && rowNum >= 0 && rowNum < 65536; rowNum += inc) { HSSFRow row = getRow(rowNum); @@ -1453,7 +1475,7 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet { // Remove all the old cells from the row we'll - // be writing too, before we start overwriting + // be writing to, before we start overwriting // any cells. This avoids issues with cells // changing type, and records not being correctly // overwritten @@ -1475,13 +1497,13 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet { // the destination row for (Iterator cells = row.cellIterator(); cells.hasNext(); ) { HSSFCell cell = (HSSFCell) cells.next(); + HSSFHyperlink link = cell.getHyperlink(); row.removeCell(cell); CellValueRecordInterface cellRecord = cell.getCellValueRecord(); cellRecord.setRow(rowNum + n); row2Replace.createCellFromRecord(cellRecord); _sheet.addValueRecord(rowNum + n, cellRecord); - HSSFHyperlink link = cell.getHyperlink(); if (link != null) { link.setFirstRow(link.getFirstRow() + n); link.setLastRow(link.getLastRow() + n); @@ -2058,6 +2080,35 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet { } return hyperlinkList; } + + /** + * Remove the underlying HyperlinkRecord from this sheet. + * If multiple HSSFHyperlinks refer to the same HyperlinkRecord, all HSSFHyperlinks will be removed. + * + * @param link the HSSFHyperlink wrapper around the HyperlinkRecord to remove + */ + protected void removeHyperlink(HSSFHyperlink link) { + removeHyperlink(link.record); + } + + /** + * Remove the underlying HyperlinkRecord from this sheet + * + * @param link the underlying HyperlinkRecord to remove from this sheet + */ + protected void removeHyperlink(HyperlinkRecord link) { + for (Iterator it = _sheet.getRecords().iterator(); it.hasNext();) { + RecordBase rec = it.next(); + if (rec instanceof HyperlinkRecord) { + HyperlinkRecord recLink = (HyperlinkRecord) rec; + if (link == recLink) { + it.remove(); + // if multiple HSSFHyperlinks refer to the same record + return; + } + } + } + } public HSSFSheetConditionalFormatting getSheetConditionalFormatting() { return new HSSFSheetConditionalFormatting(this); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java index 159e256b6a..d683a99c35 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java @@ -23,12 +23,7 @@ import java.io.IOException; import org.apache.poi.ss.usermodel.BaseTestSheetShiftRows; import org.apache.poi.ss.usermodel.Cell; -import org.apache.poi.ss.usermodel.CellStyle; import org.apache.poi.ss.usermodel.Comment; -import org.apache.poi.ss.usermodel.CreationHelper; -import org.apache.poi.ss.usermodel.Font; -import org.apache.poi.ss.usermodel.Hyperlink; -import org.apache.poi.ss.usermodel.IndexedColors; import org.apache.poi.ss.usermodel.Row; import org.apache.poi.ss.usermodel.Sheet; import org.apache.poi.ss.usermodel.Workbook; @@ -399,106 +394,4 @@ public final class TestXSSFSheetShiftRows extends BaseTestSheetShiftRows { wb.close(); } - - @Test - public void testBug46742_shiftHyperlinks() throws IOException { - XSSFWorkbook wb = new XSSFWorkbook(); - Sheet sheet = wb.createSheet("test"); - Row row = sheet.createRow(0); - - // How to create hyperlinks - // https://poi.apache.org/spreadsheet/quick-guide.html#Hyperlinks - XSSFCreationHelper helper = wb.getCreationHelper(); - CellStyle hlinkStyle = wb.createCellStyle(); - Font hlinkFont = wb.createFont(); - hlinkFont.setUnderline(Font.U_SINGLE); - hlinkFont.setColor(IndexedColors.BLUE.getIndex()); - hlinkStyle.setFont(hlinkFont); - - // 3D relative document link - Cell cell = row.createCell(0); - cell.setCellStyle(hlinkStyle); - createHyperlink(helper, cell, Hyperlink.LINK_DOCUMENT, "test!E1"); - - // URL - cell = row.createCell(1); - cell.setCellStyle(hlinkStyle); - createHyperlink(helper, cell, Hyperlink.LINK_URL, "http://poi.apache.org/"); - - // row0 will be shifted on top of row1, so this URL should be removed from the workbook - Row overwrittenRow = sheet.createRow(3); - cell = overwrittenRow.createCell(2); - cell.setCellStyle(hlinkStyle); - createHyperlink(helper, cell, Hyperlink.LINK_EMAIL, "mailto:poi@apache.org"); - - // hyperlinks on this row are unaffected by the row shifting, so the hyperlinks should not move - Row unaffectedRow = sheet.createRow(20); - cell = unaffectedRow.createCell(3); - cell.setCellStyle(hlinkStyle); - createHyperlink(helper, cell, Hyperlink.LINK_FILE, "54524.xlsx"); - - cell = wb.createSheet("other").createRow(0).createCell(0); - cell.setCellStyle(hlinkStyle); - createHyperlink(helper, cell, Hyperlink.LINK_URL, "http://apache.org/"); - - int startRow = 0; - int endRow = 0; - int n = 3; - sheet.shiftRows(startRow, endRow, n); - - XSSFWorkbook read = XSSFTestDataSamples.writeOutAndReadBack(wb); - wb.close(); - - XSSFSheet sh = read.getSheet("test"); - - Row shiftedRow = sh.getRow(3); - - // document link anchored on a shifted cell should be moved - // Note that hyperlinks do not track what they point to, so this hyperlink should still refer to test!E1 - verifyHyperlink(shiftedRow.getCell(0), Hyperlink.LINK_DOCUMENT, "test!E1"); - - // URL, EMAIL, and FILE links anchored on a shifted cell should be moved - verifyHyperlink(shiftedRow.getCell(1), Hyperlink.LINK_URL, "http://poi.apache.org/"); - - // Make sure hyperlinks were moved and not copied - assertNull(sh.getRow(0)); - - // Make sure hyperlink in overwritten row is deleted - assertEquals(3, sh.getHyperlinkList().size()); - for (XSSFHyperlink link : sh.getHyperlinkList()) { - if ("C4".equals(link.getCellRef())) { - fail("Row 4, including the hyperlink at C4, should have been deleted when Row 1 was shifted on top of it."); - } - } - - // Make sure unaffected rows are not shifted - Cell unaffectedCell = sh.getRow(20).getCell(3); - assertTrue(cellHasHyperlink(unaffectedCell)); - verifyHyperlink(unaffectedCell, Hyperlink.LINK_FILE, "54524.xlsx"); - - // Make sure cells on other sheets are not affected - unaffectedCell = read.getSheet("other").getRow(0).getCell(0); - assertTrue(cellHasHyperlink(unaffectedCell)); - verifyHyperlink(unaffectedCell, Hyperlink.LINK_URL, "http://apache.org/"); - - read.close(); - } - - private void createHyperlink(CreationHelper helper, Cell cell, int linkType, String ref) { - cell.setCellValue(ref); - Hyperlink link = helper.createHyperlink(linkType); - link.setAddress(ref); - cell.setHyperlink(link); - } - - private void verifyHyperlink(Cell cell, int linkType, String ref) { - assertTrue(cellHasHyperlink(cell)); - Hyperlink link = cell.getHyperlink(); - assertEquals(linkType, link.getType()); - assertEquals(ref, link.getAddress()); - } - - private boolean cellHasHyperlink(Cell cell) { - return (cell != null) && (cell.getHyperlink() != null); - } } diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java index 34ef12b7cb..37e77d1f20 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java @@ -21,11 +21,13 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.IOException; import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.ss.ITestDataProvider; +import org.apache.poi.ss.util.CellAddress; import org.apache.poi.ss.util.CellRangeAddress; import org.apache.poi.ss.util.CellReference; import org.junit.Test; @@ -456,4 +458,124 @@ public abstract class BaseTestSheetShiftRows { wb.close(); } + + /** + * 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 { + Workbook wb = _testDataProvider.createWorkbook(); + Sheet sheet = wb.createSheet("test"); + Row row = sheet.createRow(0); + + // How to create hyperlinks + // https://poi.apache.org/spreadsheet/quick-guide.html#Hyperlinks + CreationHelper helper = wb.getCreationHelper(); + CellStyle hlinkStyle = wb.createCellStyle(); + Font hlinkFont = wb.createFont(); + hlinkFont.setUnderline(Font.U_SINGLE); + hlinkFont.setColor(IndexedColors.BLUE.getIndex()); + hlinkStyle.setFont(hlinkFont); + + // 3D relative document link + // CellAddress=A1, shifted to A4 + Cell cell = row.createCell(0); + cell.setCellStyle(hlinkStyle); + createHyperlink(helper, cell, Hyperlink.LINK_DOCUMENT, "test!E1"); + + // URL + cell = row.createCell(1); + // CellAddress=B1, shifted to B4 + cell.setCellStyle(hlinkStyle); + createHyperlink(helper, cell, Hyperlink.LINK_URL, "http://poi.apache.org/"); + + // row0 will be shifted on top of row1, so this URL should be removed from the workbook + Row overwrittenRow = sheet.createRow(3); + cell = overwrittenRow.createCell(2); + // CellAddress=C4, will be overwritten (deleted) + cell.setCellStyle(hlinkStyle); + createHyperlink(helper, cell, Hyperlink.LINK_EMAIL, "mailto:poi@apache.org"); + + // hyperlinks on this row are unaffected by the row shifting, so the hyperlinks should not move + Row unaffectedRow = sheet.createRow(20); + cell = unaffectedRow.createCell(3); + // CellAddress=D21, will be unaffected + cell.setCellStyle(hlinkStyle); + createHyperlink(helper, cell, Hyperlink.LINK_FILE, "54524.xlsx"); + + cell = wb.createSheet("other").createRow(0).createCell(0); + // CellAddress=Other!A1, will be unaffected + cell.setCellStyle(hlinkStyle); + createHyperlink(helper, cell, Hyperlink.LINK_URL, "http://apache.org/"); + + int startRow = 0; + int endRow = 0; + int n = 3; + sheet.shiftRows(startRow, endRow, n); + + Workbook read = _testDataProvider.writeOutAndReadBack(wb); + wb.close(); + + Sheet sh = read.getSheet("test"); + + Row shiftedRow = sh.getRow(3); + + // document link anchored on a shifted cell should be moved + // Note that hyperlinks do not track what they point to, so this hyperlink should still refer to test!E1 + verifyHyperlink(shiftedRow.getCell(0), Hyperlink.LINK_DOCUMENT, "test!E1"); + + // URL, EMAIL, and FILE links anchored on a shifted cell should be moved + verifyHyperlink(shiftedRow.getCell(1), Hyperlink.LINK_URL, "http://poi.apache.org/"); + + // Make sure hyperlinks were moved and not copied + assertNull("Document hyperlink should be moved, not copied", sh.getHyperlink(0, 0)); + assertNull("URL hyperlink should be moved, not copied", sh.getHyperlink(0, 1)); + + // Make sure hyperlink in overwritten row is deleted + System.out.println(sh.getHyperlinkList()); + assertEquals(3, sh.getHyperlinkList().size()); + CellAddress unexpectedLinkAddress = new CellAddress("C4"); + for (Hyperlink link : sh.getHyperlinkList()) { + final CellAddress linkAddress = new CellAddress(link.getFirstRow(), link.getFirstColumn()); + System.out.println(linkAddress.formatAsString()); + if (linkAddress.equals(unexpectedLinkAddress)) { + fail("Row 4, including the hyperlink at C4, should have " + + "been deleted when Row 1 was shifted on top of it."); + } + } + + // Make sure unaffected rows are not shifted + Cell unaffectedCell = sh.getRow(20).getCell(3); + assertTrue(cellHasHyperlink(unaffectedCell)); + verifyHyperlink(unaffectedCell, Hyperlink.LINK_FILE, "54524.xlsx"); + + // Make sure cells on other sheets are not affected + unaffectedCell = read.getSheet("other").getRow(0).getCell(0); + assertTrue(cellHasHyperlink(unaffectedCell)); + verifyHyperlink(unaffectedCell, Hyperlink.LINK_URL, "http://apache.org/"); + + read.close(); + } + + private void createHyperlink(CreationHelper helper, Cell cell, int linkType, String ref) { + cell.setCellValue(ref); + Hyperlink link = helper.createHyperlink(linkType); + link.setAddress(ref); + cell.setHyperlink(link); + } + + private void verifyHyperlink(Cell cell, int linkType, String ref) { + assertTrue(cellHasHyperlink(cell)); + Hyperlink link = cell.getHyperlink(); + assertEquals(linkType, link.getType()); + assertEquals(ref, link.getAddress()); + } + + private boolean cellHasHyperlink(Cell cell) { + return (cell != null) && (cell.getHyperlink() != null); + } } -- 2.39.5