From 16a9eca7c86ef1e55aad572aadf461b89c66903b Mon Sep 17 00:00:00 2001 From: David North Date: Fri, 12 Jun 2015 15:20:10 +0000 Subject: [PATCH] My own patch to fix #56328 Add tests for initial patch applied by Nick in r1582892 Fix and test AreaReference#isWholeColumnReference to take account of the prevailing spreadsheet version. Fixing all users of this is a big undertaking, so for now I've left a fallback on the original behaviour for some cases. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1685101 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/ss/formula/FormulaParser.java | 6 +-- .../poi/ss/formula/ptg/AreaPtgBase.java | 3 +- .../org/apache/poi/ss/util/AreaReference.java | 30 ++++++++--- .../poi/ss/formula/TestFormulaParser.java | 48 ++++++++++++++++++ .../apache/poi/ss/util/TestAreaReference.java | 50 +++++++++++++++++++ 5 files changed, 125 insertions(+), 12 deletions(-) create mode 100644 src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java create mode 100644 src/testcases/org/apache/poi/ss/util/TestAreaReference.java diff --git a/src/java/org/apache/poi/ss/formula/FormulaParser.java b/src/java/org/apache/poi/ss/formula/FormulaParser.java index 72ed008cf..5124a4463 100644 --- a/src/java/org/apache/poi/ss/formula/FormulaParser.java +++ b/src/java/org/apache/poi/ss/formula/FormulaParser.java @@ -617,16 +617,16 @@ public final class FormulaParser { return new ParseNode(ptg); } - private static AreaReference createAreaRef(SimpleRangePart part1, SimpleRangePart part2) { + private AreaReference createAreaRef(SimpleRangePart part1, SimpleRangePart part2) { if (!part1.isCompatibleForArea(part2)) { throw new FormulaParseException("has incompatible parts: '" + part1.getRep() + "' and '" + part2.getRep() + "'."); } if (part1.isRow()) { - return AreaReference.getWholeRow(part1.getRep(), part2.getRep()); + return AreaReference.getWholeRow(_ssVersion, part1.getRep(), part2.getRep()); } if (part1.isColumn()) { - return AreaReference.getWholeColumn(part1.getRep(), part2.getRep()); + return AreaReference.getWholeColumn(_ssVersion, part1.getRep(), part2.getRep()); } return new AreaReference(part1.getCellReference(), part2.getCellReference()); } diff --git a/src/java/org/apache/poi/ss/formula/ptg/AreaPtgBase.java b/src/java/org/apache/poi/ss/formula/ptg/AreaPtgBase.java index 5b77b09fe..96eda8757 100644 --- a/src/java/org/apache/poi/ss/formula/ptg/AreaPtgBase.java +++ b/src/java/org/apache/poi/ss/formula/ptg/AreaPtgBase.java @@ -17,6 +17,7 @@ package org.apache.poi.ss.formula.ptg; +import org.apache.poi.ss.SpreadsheetVersion; import org.apache.poi.ss.util.AreaReference; import org.apache.poi.ss.util.CellReference; import org.apache.poi.util.BitField; @@ -256,7 +257,7 @@ public abstract class AreaPtgBase extends OperandPtg implements AreaI { CellReference topLeft = new CellReference(getFirstRow(),getFirstColumn(),!isFirstRowRelative(),!isFirstColRelative()); CellReference botRight = new CellReference(getLastRow(),getLastColumn(),!isLastRowRelative(),!isLastColRelative()); - if(AreaReference.isWholeColumnReference(topLeft, botRight)) { + if(AreaReference.isWholeColumnReference(SpreadsheetVersion.EXCEL97, topLeft, botRight)) { return (new AreaReference(topLeft, botRight)).formatAsString(); } return topLeft.formatAsString() + ":" + botRight.formatAsString(); diff --git a/src/java/org/apache/poi/ss/util/AreaReference.java b/src/java/org/apache/poi/ss/util/AreaReference.java index d59388873..b0a69135a 100644 --- a/src/java/org/apache/poi/ss/util/AreaReference.java +++ b/src/java/org/apache/poi/ss/util/AreaReference.java @@ -35,13 +35,23 @@ public class AreaReference { private final CellReference _firstCell; private final CellReference _lastCell; private final boolean _isSingleCell; + private SpreadsheetVersion _version; + /** + * @deprecated Prefer supplying a version. + */ + @Deprecated + public AreaReference(String reference) { + this(reference, SpreadsheetVersion.EXCEL97); + } + /** * Create an area ref from a string representation. Sheet names containing special characters should be * delimited and escaped as per normal syntax rules for formulas.
* The area reference must be contiguous (i.e. represent a single rectangle, not a union of rectangles) */ - public AreaReference(String reference) { + public AreaReference(String reference, SpreadsheetVersion version) { + _version = version; if(! isContiguous(reference)) { throw new IllegalArgumentException( "References passed to the AreaReference must be contiguous, " + @@ -169,30 +179,34 @@ public class AreaReference { return false; } - public static AreaReference getWholeRow(String start, String end) { - return new AreaReference("$A" + start + ":$IV" + end); + public static AreaReference getWholeRow(SpreadsheetVersion version, String start, String end) { + return new AreaReference("$A" + start + ":$" + version.getLastColumnName() + end, version); } - public static AreaReference getWholeColumn(String start, String end) { - return new AreaReference(start + "$1:" + end + "$65536"); + public static AreaReference getWholeColumn(SpreadsheetVersion version, String start, String end) { + return new AreaReference(start + "$1:" + end + "$" + version.getMaxRows(), version); } /** * Is the reference for a whole-column reference, * such as C:C or D:G ? */ - public static boolean isWholeColumnReference(CellReference topLeft, CellReference botRight) { + public static boolean isWholeColumnReference(SpreadsheetVersion version, CellReference topLeft, CellReference botRight) { + if (null == version) { + version = SpreadsheetVersion.EXCEL97; // how the code used to behave. + } + // These are represented as something like // C$1:C$65535 or D$1:F$0 // i.e. absolute from 1st row to 0th one if(topLeft.getRow() == 0 && topLeft.isRowAbsolute() && - botRight.getRow() == SpreadsheetVersion.EXCEL97.getLastRowIndex() && botRight.isRowAbsolute()) { + botRight.getRow() == version.getLastRowIndex() && botRight.isRowAbsolute()) { return true; } return false; } public boolean isWholeColumnReference() { - return isWholeColumnReference(_firstCell, _lastCell); + return isWholeColumnReference(_version, _firstCell, _lastCell); } /** diff --git a/src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java b/src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java new file mode 100644 index 000000000..478ad13bf --- /dev/null +++ b/src/ooxml/testcases/org/apache/poi/ss/formula/TestFormulaParser.java @@ -0,0 +1,48 @@ +package org.apache.poi.ss.formula; + +import org.apache.poi.hssf.usermodel.HSSFEvaluationWorkbook; +import org.apache.poi.hssf.usermodel.HSSFWorkbook; +import org.apache.poi.xssf.usermodel.XSSFEvaluationWorkbook; +import org.apache.poi.xssf.usermodel.XSSFWorkbook; + +import junit.framework.TestCase; + +/** + * Test {@link FormulaParser}'s handling of row numbers at the edge of the + * HSSF/XSSF ranges. + * + * @author David North + */ +public class TestFormulaParser extends TestCase { + + public void testHSSFFailsForOver65536() { + FormulaParsingWorkbook workbook = HSSFEvaluationWorkbook.create(new HSSFWorkbook()); + try { + FormulaParser.parse("Sheet1!1:65537", workbook, FormulaType.CELL, 0); + fail("Expected exception"); + } + catch (FormulaParseException expected) { + } + } + + public void testHSSFPassCase() { + FormulaParsingWorkbook workbook = HSSFEvaluationWorkbook.create(new HSSFWorkbook()); + FormulaParser.parse("Sheet1!1:65536", workbook, FormulaType.CELL, 0); + } + + public void testXSSFWorksForOver65536() { + FormulaParsingWorkbook workbook = XSSFEvaluationWorkbook.create(new XSSFWorkbook()); + FormulaParser.parse("Sheet1!1:65537", workbook, FormulaType.CELL, 0); + } + + public void testXSSFFailCase() { + FormulaParsingWorkbook workbook = XSSFEvaluationWorkbook.create(new XSSFWorkbook()); + try { + FormulaParser.parse("Sheet1!1:1048577", workbook, FormulaType.CELL, 0); // one more than max rows. + fail("Expected exception"); + } + catch (FormulaParseException expected) { + } + } + +} diff --git a/src/testcases/org/apache/poi/ss/util/TestAreaReference.java b/src/testcases/org/apache/poi/ss/util/TestAreaReference.java new file mode 100644 index 000000000..43158038b --- /dev/null +++ b/src/testcases/org/apache/poi/ss/util/TestAreaReference.java @@ -0,0 +1,50 @@ +package org.apache.poi.ss.util; + +import org.apache.poi.ss.SpreadsheetVersion; + +import junit.framework.TestCase; + +/** + * Test for {@link AreaReference} handling of max rows. + * + * @author David North + */ +public class TestAreaReference extends TestCase { + + public void testWholeColumn() { + AreaReference oldStyle = AreaReference.getWholeColumn(SpreadsheetVersion.EXCEL97, "A", "B"); + assertEquals(0, oldStyle.getFirstCell().getCol()); + assertEquals(0, oldStyle.getFirstCell().getRow()); + assertEquals(1, oldStyle.getLastCell().getCol()); + assertEquals(SpreadsheetVersion.EXCEL97.getLastRowIndex(), oldStyle.getLastCell().getRow()); + assertTrue(oldStyle.isWholeColumnReference()); + + AreaReference oldStyleNonWholeColumn = new AreaReference("A1:B23", SpreadsheetVersion.EXCEL97); + assertFalse(oldStyleNonWholeColumn.isWholeColumnReference()); + + AreaReference newStyle = AreaReference.getWholeColumn(SpreadsheetVersion.EXCEL2007, "A", "B"); + assertEquals(0, newStyle.getFirstCell().getCol()); + assertEquals(0, newStyle.getFirstCell().getRow()); + assertEquals(1, newStyle.getLastCell().getCol()); + assertEquals(SpreadsheetVersion.EXCEL2007.getLastRowIndex(), newStyle.getLastCell().getRow()); + assertTrue(newStyle.isWholeColumnReference()); + + AreaReference newStyleNonWholeColumn = new AreaReference("A1:B23", SpreadsheetVersion.EXCEL2007); + assertFalse(newStyleNonWholeColumn.isWholeColumnReference()); + } + + public void testWholeRow() { + AreaReference oldStyle = AreaReference.getWholeRow(SpreadsheetVersion.EXCEL97, "1", "2"); + assertEquals(0, oldStyle.getFirstCell().getCol()); + assertEquals(0, oldStyle.getFirstCell().getRow()); + assertEquals(SpreadsheetVersion.EXCEL97.getLastColumnIndex(), oldStyle.getLastCell().getCol()); + assertEquals(1, oldStyle.getLastCell().getRow()); + + AreaReference newStyle = AreaReference.getWholeRow(SpreadsheetVersion.EXCEL2007, "1", "2"); + assertEquals(0, newStyle.getFirstCell().getCol()); + assertEquals(0, newStyle.getFirstCell().getRow()); + assertEquals(SpreadsheetVersion.EXCEL2007.getLastColumnIndex(), newStyle.getLastCell().getCol()); + assertEquals(1, newStyle.getLastCell().getRow()); + } + +}