From e501d4015df167e69688635df8092f993d5a9094 Mon Sep 17 00:00:00 2001 From: Vladislav Galas Date: Wed, 2 Jan 2019 22:08:38 +0000 Subject: [PATCH] Bug 62828: CellReference(Cell) now initializes sheet name. Changed CellReference to CellAddress in XSSFHyperlink because it is what it should return. Updated all relevant tests, added a test for CellReference(Cell). git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1850210 13f79535-47bb-0310-9956-ffa450edef68 --- build.gradle | 1 + .../org/apache/poi/ss/util/CellReference.java | 2 +- .../apache/poi/xssf/usermodel/XSSFCell.java | 2 +- .../poi/xssf/usermodel/XSSFHyperlink.java | 3 +-- .../apache/poi/xssf/usermodel/XSSFSheet.java | 2 +- .../poi/xssf/usermodel/TestXSSFCell.java | 13 +++++----- .../poi/xssf/usermodel/TestXSSFSheet.java | 4 +-- .../BaseTestSheetUpdateArrayFormulas.java | 8 +++--- .../apache/poi/ss/util/TestCellReference.java | 25 +++++++++++++++++++ 9 files changed, 43 insertions(+), 17 deletions(-) diff --git a/build.gradle b/build.gradle index 9c4afa814b..cdf4f1767e 100644 --- a/build.gradle +++ b/build.gradle @@ -185,6 +185,7 @@ project('main') { compile 'javax.activation:activation:1.1.1' testCompile 'junit:junit:4.12' + testCompile 'org.mockito:mockito-core:2.21.0' testCompile 'org.reflections:reflections:0.9.11' } diff --git a/src/java/org/apache/poi/ss/util/CellReference.java b/src/java/org/apache/poi/ss/util/CellReference.java index 3ef32e30f6..be2737a1c1 100644 --- a/src/java/org/apache/poi/ss/util/CellReference.java +++ b/src/java/org/apache/poi/ss/util/CellReference.java @@ -145,7 +145,7 @@ public class CellReference { } public CellReference(Cell cell) { - this(cell.getRowIndex(), cell.getColumnIndex(), false, false); + this(cell.getSheet().getSheetName(), cell.getRowIndex(), cell.getColumnIndex(), false, false); } public CellReference(int pRow, int pCol, boolean pAbsRow, boolean pAbsCol) { diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java index a1a66bf653..864b031729 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java @@ -1295,7 +1295,7 @@ public final class XSSFCell implements Cell { public CellRangeAddress getArrayFormulaRange() { XSSFCell cell = getSheet().getFirstCellInArrayFormula(this); if (cell == null) { - throw new IllegalStateException("Cell " + getReference() + throw new IllegalStateException("Cell " + new CellReference(this).formatAsString() + " is not part of an array formula."); } String formulaRef = cell._cell.getF().getRef(); 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 be7f6c397c..c02e00ae78 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFHyperlink.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFHyperlink.java @@ -177,8 +177,7 @@ public class XSSFHyperlink implements Hyperlink { } /** - * Get the reference of the cell this applies to, - * es A55 + * Get the address of the cell this hyperlink applies to, e.g. A55 */ public String getCellRef() { return _ctHyperlink.getRef(); 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 bbd7eda838..555968c42a 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java @@ -3996,7 +3996,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet { return cr; } } - String ref = ((XSSFCell)cell).getCTCell().getR(); + String ref = new CellReference(cell).formatAsString(); throw new IllegalArgumentException("Cell " + ref + " is not part of an array formula."); } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCell.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCell.java index d69ab95cfb..7d78848a29 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCell.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCell.java @@ -40,6 +40,7 @@ import org.apache.poi.ss.usermodel.Row; import org.apache.poi.ss.usermodel.Row.MissingCellPolicy; import org.apache.poi.ss.usermodel.Sheet; import org.apache.poi.ss.usermodel.Workbook; +import org.apache.poi.ss.util.CellAddress; import org.apache.poi.ss.util.CellRangeAddress; import org.apache.poi.ss.util.CellReference; import org.apache.poi.xssf.SXSSFITestDataProvider; @@ -597,9 +598,9 @@ public final class TestXSSFCell extends BaseTestXCell { final List links = srcCell.getSheet().getHyperlinkList(); assertEquals("number of hyperlinks on sheet", 2, links.size()); assertEquals("source hyperlink", - new CellReference(srcCell).formatAsString(), links.get(0).getCellRef()); + new CellAddress(srcCell).formatAsString(), links.get(0).getCellRef()); assertEquals("destination hyperlink", - new CellReference(destCell).formatAsString(), links.get(1).getCellRef()); + new CellAddress(destCell).formatAsString(), links.get(1).getCellRef()); wb.close(); } @@ -639,7 +640,7 @@ public final class TestXSSFCell extends BaseTestXCell { links = srcCell.getSheet().getHyperlinkList(); assertEquals("number of hyperlinks on sheet", 1, links.size()); assertEquals("source hyperlink", - new CellReference(destCell).formatAsString(), links.get(0).getCellRef()); + new CellAddress(destCell).formatAsString(), links.get(0).getCellRef()); // Merge destCell's hyperlink to srcCell. Since destCell does have a hyperlink, this should copy destCell's hyperlink to srcCell. srcCell.copyCellFrom(destCell, policy); @@ -649,9 +650,9 @@ public final class TestXSSFCell extends BaseTestXCell { links = srcCell.getSheet().getHyperlinkList(); assertEquals("number of hyperlinks on sheet", 2, links.size()); assertEquals("dest hyperlink", - new CellReference(destCell).formatAsString(), links.get(0).getCellRef()); + new CellAddress(destCell).formatAsString(), links.get(0).getCellRef()); assertEquals("source hyperlink", - new CellReference(srcCell).formatAsString(), links.get(1).getCellRef()); + new CellAddress(srcCell).formatAsString(), links.get(1).getCellRef()); wb.close(); } @@ -745,4 +746,4 @@ public final class TestXSSFCell extends BaseTestXCell { } } -} \ No newline at end of file +} diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java index de828ba1c9..85118916c8 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java @@ -1506,7 +1506,7 @@ public final class TestXSSFSheet extends BaseTestXSheet { // Cell Formula cell = CellUtil.getCell(destRow, col++); - assertEquals("J7", new CellReference(cell).formatAsString()); + assertEquals("Sheet1!J7", new CellReference(cell).formatAsString()); assertEquals("[Cell Formula] J7 cell type", CellType.FORMULA, cell.getCellType()); assertEquals("[Cell Formula] J7 cell formula", "5+2", cell.getCellFormula()); //System.out.println("Cell formula evaluation currently unsupported"); @@ -1514,7 +1514,7 @@ public final class TestXSSFSheet extends BaseTestXSheet { // Cell Formula with Reference // Formula row references should be adjusted by destRowNum-srcRowNum cell = CellUtil.getCell(destRow, col++); - assertEquals("K7", new CellReference(cell).formatAsString()); + assertEquals("Sheet1!K7", new CellReference(cell).formatAsString()); assertEquals("[Cell Formula with Reference] K7 cell type", CellType.FORMULA, cell.getCellType()); assertEquals("[Cell Formula with Reference] K7 cell formula", diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetUpdateArrayFormulas.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetUpdateArrayFormulas.java index a076c1685c..9b20c6b3c6 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetUpdateArrayFormulas.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetUpdateArrayFormulas.java @@ -86,7 +86,7 @@ public abstract class BaseTestSheetUpdateArrayFormulas { cell.getArrayFormulaRange(); fail("expected exception"); } catch (IllegalStateException e){ - assertEquals("Cell A1 is not part of an array formula.", e.getMessage()); + assertEquals("Cell Sheet0!A1 is not part of an array formula.", e.getMessage()); } // row 3 does not yet exist @@ -177,14 +177,14 @@ public abstract class BaseTestSheetUpdateArrayFormulas { cell.getArrayFormulaRange(); fail("expected exception"); } catch (IllegalStateException e){ - assertEquals("Cell A1 is not part of an array formula.", e.getMessage()); + assertEquals("Cell Sheet0!A1 is not part of an array formula.", e.getMessage()); } try { sheet.removeArrayFormula(cell); fail("expected exception"); } catch (IllegalArgumentException e){ - assertEquals("Cell A1 is not part of an array formula.", e.getMessage()); + assertEquals("Cell Sheet0!A1 is not part of an array formula.", e.getMessage()); } workbook.close(); @@ -221,7 +221,7 @@ public abstract class BaseTestSheetUpdateArrayFormulas { fail("expected exception"); } catch (IllegalArgumentException e){ String ref = new CellReference(acell).formatAsString(); - assertEquals("Cell "+ref+" is not part of an array formula.", e.getMessage()); + assertEquals("Cell " + ref + " is not part of an array formula.", e.getMessage()); } } diff --git a/src/testcases/org/apache/poi/ss/util/TestCellReference.java b/src/testcases/org/apache/poi/ss/util/TestCellReference.java index a39bd0b725..103d3926fb 100644 --- a/src/testcases/org/apache/poi/ss/util/TestCellReference.java +++ b/src/testcases/org/apache/poi/ss/util/TestCellReference.java @@ -19,12 +19,16 @@ package org.apache.poi.ss.util; import org.apache.poi.ss.SpreadsheetVersion; +import org.apache.poi.ss.usermodel.Cell; import org.junit.Test; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.RETURNS_DEEP_STUBS; /** * Tests that the common CellReference works as we need it to. @@ -56,6 +60,27 @@ public final class TestCellReference { cellReference = new CellReference(sheet, row, col, absRow, absCol); assertEquals("Sheet1!A$1", cellReference.formatAsString()); + + cellReference = new CellReference(sheet+"!$A1"); + assertFalse(cellReference.isRowAbsolute()); + assertTrue(cellReference.isColAbsolute()); + + cellReference = new CellReference(sheet+"!A$1"); + assertTrue(cellReference.isRowAbsolute()); + assertFalse(cellReference.isColAbsolute()); + } + + @Test + public void testCtorFromCell() { + Cell cell = mock(Cell.class, RETURNS_DEEP_STUBS); + when(cell.getSheet().getSheetName()).thenReturn("sheet"); + + CellReference result = new CellReference(cell); + assertEquals("sheet", result.getSheetName()); + assertEquals(cell.getRowIndex(), result.getRow()); + assertEquals(cell.getColumnIndex(), result.getCol()); + assertFalse(result.isRowAbsolute()); + assertFalse(result.isColAbsolute()); } @Test -- 2.39.5