From 139673846086de2d705f8fa4f1bd7c09dede6634 Mon Sep 17 00:00:00 2001 From: Javen O'Neal Date: Thu, 29 Oct 2015 06:53:55 +0000 Subject: [PATCH] bug 58557: fix from Alessandro Guarascio, shift hyperlinks when shifting rows on an XSSFSheet git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1711185 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/xssf/usermodel/XSSFHyperlink.java | 9 +- .../apache/poi/xssf/usermodel/XSSFSheet.java | 32 ++++++ .../usermodel/helpers/XSSFRowShifter.java | 26 +++++ .../usermodel/TestXSSFSheetShiftRows.java | 106 ++++++++++++++++++ 4 files changed, 170 insertions(+), 3 deletions(-) diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFHyperlink.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFHyperlink.java index 86538fd68..b338e3eed 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFHyperlink.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFHyperlink.java @@ -23,6 +23,7 @@ import org.apache.poi.openxml4j.opc.PackagePart; import org.apache.poi.openxml4j.opc.PackageRelationship; import org.apache.poi.ss.usermodel.Hyperlink; import org.apache.poi.ss.util.CellReference; +import org.apache.poi.util.Internal; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTHyperlink; /** @@ -33,8 +34,8 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTHyperlink; public class XSSFHyperlink implements Hyperlink { private int _type; private PackageRelationship _externalRel; - private CTHyperlink _ctHyperlink; - private String _location; + private CTHyperlink _ctHyperlink; //contains a reference to the cell where the hyperlink is anchored, getRef() + private String _location; //what the hyperlink refers to /** * Create a new XSSFHyperlink. This method is protected to be used only by XSSFCreationHelper @@ -94,6 +95,7 @@ public class XSSFHyperlink implements Hyperlink { /** * @return the underlying CTHyperlink object */ + @Internal public CTHyperlink getCTHyperlink() { return _ctHyperlink; } @@ -219,7 +221,8 @@ public class XSSFHyperlink implements Hyperlink { /** * Assigns this hyperlink to the given cell reference */ - protected void setCellReference(String ref) { + @Internal + public void setCellReference(String ref) { _ctHyperlink.setRef(ref); } protected void setCellReference(CellReference ref) { 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 d27a96e7c..832c987f1 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java @@ -25,6 +25,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.Iterator; @@ -669,6 +670,13 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet { vml == null ? null : vml.findCommentShape(row, column)); } + /** + * Get a Hyperlink in this sheet anchored at row, column + * + * @param row + * @param column + * @return hyperlink if there is a hyperlink anchored at row, column; otherwise returns null + */ public XSSFHyperlink getHyperlink(int row, int column) { String ref = new CellReference(row, column).formatAsString(); for(XSSFHyperlink hyperlink : hyperlinks) { @@ -678,6 +686,15 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet { } return null; } + + /** + * Get a list of Hyperlinks in this sheet + * + * @return + */ + public List getHyperlinkList() { + return Collections.unmodifiableList(hyperlinks); + } @SuppressWarnings("deprecation") private int[] getBreaks(CTPageBreak ctPageBreak) { @@ -2610,6 +2627,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet { // remove row from _rows it.remove(); + // FIXME: (performance optimization) this should be moved outside the for-loop so that comments only needs to be iterated over once. // also remove any comments associated with this row if(sheetComments != null){ CTCommentList lst = sheetComments.getCTComments().getCommentList(); @@ -2624,6 +2642,16 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet { } } } + // FIXME: (performance optimization) this should be moved outside the for-loop so that hyperlinks only needs to be iterated over once. + // also remove any hyperlinks associated with this row + if (hyperlinks != null) { + for (XSSFHyperlink link : new ArrayList(hyperlinks)) { + CellReference ref = new CellReference(link.getCellRef()); + if (ref.getRow() == rownum) { + hyperlinks.remove(link); + } + } + } } } @@ -2707,6 +2735,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet { rowShifter.updateFormulas(shifter); rowShifter.shiftMerged(startRow, endRow, n); rowShifter.updateConditionalFormatting(shifter); + rowShifter.updateHyperlinks(shifter); //rebuild the _rows map SortedMap map = new TreeMap(); @@ -2902,6 +2931,9 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet { */ @Internal public void removeHyperlink(int row, int column) { + // CTHyperlinks is regenerated from scratch when writing out the spreadsheet + // so don't worry about maintaining hyperlinks and CTHyperlinks in parallel. + // only maintain hyperlinks String ref = new CellReference(row, column).formatAsString(); for (Iterator it = hyperlinks.iterator(); it.hasNext();) { XSSFHyperlink hyperlink = it.next(); diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java index 075320c02..02eabd46e 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java @@ -22,6 +22,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import org.apache.poi.common.usermodel.Hyperlink; import org.apache.poi.ss.formula.FormulaParseException; import org.apache.poi.ss.formula.FormulaParser; import org.apache.poi.ss.formula.FormulaRenderer; @@ -38,6 +39,7 @@ import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; import org.apache.poi.xssf.usermodel.XSSFCell; import org.apache.poi.xssf.usermodel.XSSFEvaluationWorkbook; +import org.apache.poi.xssf.usermodel.XSSFHyperlink; import org.apache.poi.xssf.usermodel.XSSFName; import org.apache.poi.xssf.usermodel.XSSFRow; import org.apache.poi.xssf.usermodel.XSSFSheet; @@ -280,6 +282,30 @@ public final class XSSFRowShifter { } } } + + /** + * Shift the Hyperlink anchors (not the hyperlink text, even if the hyperlink + * is of type LINK_DOCUMENT and refers to a cell that was shifted). Hyperlinks + * do not track the content they point to. + * + * @param shifter + */ + public void updateHyperlinks(FormulaShifter shifter) { + int sheetIndex = sheet.getWorkbook().getSheetIndex(sheet); + List hyperlinkList = sheet.getHyperlinkList(); + + for (XSSFHyperlink hyperlink : hyperlinkList) { + String cellRef = hyperlink.getCellRef(); + CellRangeAddress cra = CellRangeAddress.valueOf(cellRef); + CellRangeAddress shiftedRange = shiftRange(shifter, cra, sheetIndex); + if (shiftedRange != null && shiftedRange != cra) { + // shiftedRange should not be null. If shiftedRange is null, that means + // that a hyperlink wasn't deleted at the beginning of shiftRows when + // identifying rows that should be removed because they will be overwritten + hyperlink.setCellReference(shiftedRange.formatAsString()); + } + } + } private static CellRangeAddress shiftRange(FormulaShifter shifter, CellRangeAddress cra, int currentExternSheetIx) { // FormulaShifter works well in terms of Ptgs - so convert CellRangeAddress to AreaPtg (and back) here 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 89364463b..d1d948d33 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java @@ -21,7 +21,12 @@ 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; @@ -366,4 +371,105 @@ public final class TestXSSFSheetShiftRows extends BaseTestSheetShiftRows { wb.close(); } + + 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); + } }