You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by on...@apache.org on 2016/06/13 08:58:55 UTC
svn commit: r1748144 - in /poi/trunk/src:
java/org/apache/poi/ss/util/CellReference.java
testcases/org/apache/poi/ss/util/TestCellReference.java
Author: onealj
Date: Mon Jun 13 08:58:55 2016
New Revision: 1748144
URL: http://svn.apache.org/viewvc?rev=1748144&view=rev
Log:
improve unit test coverage on CellReference.
Add note that _sheetName can be null or empty string, depending on the entry-point.
Throw an exception when parsing a string if sheet name is not quoted and contains a space.
Modified:
poi/trunk/src/java/org/apache/poi/ss/util/CellReference.java
poi/trunk/src/testcases/org/apache/poi/ss/util/TestCellReference.java
Modified: poi/trunk/src/java/org/apache/poi/ss/util/CellReference.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/util/CellReference.java?rev=1748144&r1=1748143&r2=1748144&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/ss/util/CellReference.java (original)
+++ poi/trunk/src/java/org/apache/poi/ss/util/CellReference.java Mon Jun 13 08:58:55 2016
@@ -91,9 +91,11 @@ public class CellReference {
//private static final String BIFF8_LAST_ROW = String.valueOf(SpreadsheetVersion.EXCEL97.getMaxRows());
//private static final int BIFF8_LAST_ROW_TEXT_LEN = BIFF8_LAST_ROW.length();
+ // FIXME: _sheetName may be null, depending on the entry point.
+ // Perhaps it would be better to declare _sheetName is never null, using an empty string to represent a 2D reference.
+ private final String _sheetName;
private final int _rowIndex;
private final int _colIndex;
- private final String _sheetName;
private final boolean _isRowAbs;
private final boolean _isColAbs;
@@ -353,17 +355,8 @@ public class CellReference {
}
public static boolean isRowWithinRange(String rowStr, SpreadsheetVersion ssVersion) {
- int rowNum = Integer.parseInt(rowStr);
-
- if (rowNum < 0) {
- throw new IllegalStateException("Invalid rowStr '" + rowStr + "'.");
- }
- if (rowNum == 0) {
- // execution gets here because caller does first pass of discriminating
- // potential cell references using a simplistic regex pattern.
- return false;
- }
- return rowNum <= ssVersion.getMaxRows();
+ int rowNum = Integer.parseInt(rowStr) - 1;
+ return 0 <= rowNum && rowNum <= ssVersion.getLastRowIndex();
}
private static final class CellRefParts {
@@ -408,11 +401,16 @@ public class CellReference {
boolean isQuoted = reference.charAt(0) == SPECIAL_NAME_DELIMITER;
if(!isQuoted) {
- return reference.substring(0, indexOfSheetNameDelimiter);
+ // sheet names with spaces must be quoted
+ if (reference.indexOf(' ') == -1) {
+ return reference.substring(0, indexOfSheetNameDelimiter);
+ } else {
+ throw new IllegalArgumentException("Sheet names containing spaces must be quoted: (" + reference + ")");
+ }
}
int lastQuotePos = indexOfSheetNameDelimiter-1;
if(reference.charAt(lastQuotePos) != SPECIAL_NAME_DELIMITER) {
- throw new RuntimeException("Mismatched quotes: (" + reference + ")");
+ throw new IllegalArgumentException("Mismatched quotes: (" + reference + ")");
}
// TODO - refactor cell reference parsing logic to one place.
@@ -438,7 +436,7 @@ public class CellReference {
continue;
}
}
- throw new RuntimeException("Bad sheet name quote escaping: (" + reference + ")");
+ throw new IllegalArgumentException("Bad sheet name quote escaping: (" + reference + ")");
}
return sb.toString();
}
@@ -555,7 +553,10 @@ public class CellReference {
return _rowIndex == cr._rowIndex
&& _colIndex == cr._colIndex
&& _isRowAbs == cr._isRowAbs
- && _isColAbs == cr._isColAbs;
+ && _isColAbs == cr._isColAbs
+ && ((_sheetName == null)
+ ? (cr._sheetName == null)
+ : _sheetName.equals(cr._sheetName));
}
@Override
@@ -565,6 +566,7 @@ public class CellReference {
result = 31 * result + _colIndex;
result = 31 * result + (_isRowAbs ? 1 : 0);
result = 31 * result + (_isColAbs ? 1 : 0);
+ result = 31 * result + (_sheetName == null ? 0 : _sheetName.hashCode());
return result;
}
}
Modified: poi/trunk/src/testcases/org/apache/poi/ss/util/TestCellReference.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/util/TestCellReference.java?rev=1748144&r1=1748143&r2=1748144&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/ss/util/TestCellReference.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/ss/util/TestCellReference.java Mon Jun 13 08:58:55 2016
@@ -317,4 +317,90 @@ public final class TestCellReference {
assertTrue("row absolute", ref.isRowAbsolute());
//assertFalse("column absolute/relative is undefined", ref.isColAbsolute());
}
+
+ @Test
+ public void getSheetName() {
+ assertEquals(null, new CellReference("A5").getSheetName());
+ assertEquals(null, new CellReference(null, 0, 0, false, false).getSheetName());
+ // FIXME: CellReference is inconsistent
+ assertEquals("", new CellReference("", 0, 0, false, false).getSheetName());
+ assertEquals("Sheet1", new CellReference("Sheet1!A5").getSheetName());
+ assertEquals("Sheet 1", new CellReference("'Sheet 1'!A5").getSheetName());
+ }
+
+ @Test
+ public void testToString() {
+ CellReference ref = new CellReference("'Sheet 1'!A5");
+ assertEquals("org.apache.poi.ss.util.CellReference ['Sheet 1'!A5]", ref.toString());
+ }
+
+ @Test
+ public void testEqualsAndHashCode() {
+ CellReference ref1 = new CellReference("'Sheet 1'!A5");
+ CellReference ref2 = new CellReference("Sheet 1", 4, 0, false, false);
+ assertEquals("equals", ref1, ref2);
+ assertEquals("hash code", ref1.hashCode(), ref2.hashCode());
+
+ assertFalse("null", ref1.equals(null));
+ assertFalse("3D vs 2D", ref1.equals(new CellReference("A5")));
+ assertFalse("type", ref1.equals(new Integer(0)));
+ }
+
+ @Test
+ public void isRowWithinRange() {
+ SpreadsheetVersion ss = SpreadsheetVersion.EXCEL2007;
+ assertFalse("1 before first row", CellReference.isRowWithinRange("0", ss));
+ assertTrue("first row", CellReference.isRowWithinRange("1", ss));
+ assertTrue("last row", CellReference.isRowWithinRange("1048576", ss));
+ assertFalse("1 beyond last row", CellReference.isRowWithinRange("1048577", ss));
+ }
+
+ @Test
+ public void isColWithinRange() {
+ SpreadsheetVersion ss = SpreadsheetVersion.EXCEL2007;
+ assertTrue("(empty)", CellReference.isColumnWithinRange("", ss));
+ assertTrue("first column (A)", CellReference.isColumnWithinRange("A", ss));
+ assertTrue("last column (XFD)", CellReference.isColumnWithinRange("XFD", ss));
+ assertFalse("1 beyond last column (XFE)", CellReference.isColumnWithinRange("XFE", ss));
+ }
+
+ @Test(expected=IllegalArgumentException.class)
+ public void unquotedSheetName() {
+ new CellReference("'Sheet 1!A5");
+ }
+ @Test(expected=IllegalArgumentException.class)
+ public void mismatchedQuotesSheetName() {
+ new CellReference("Sheet 1!A5");
+ }
+
+ @Test
+ public void escapedSheetName() {
+ String escapedName = "'Don''t Touch'!A5";
+ String unescapedName = "'Don't Touch'!A5";
+ new CellReference(escapedName);
+ try {
+ new CellReference(unescapedName);
+ fail("Sheet names containing apostrophe's must be escaped via a repeated apostrophe");
+ } catch (IllegalArgumentException e) {
+ assertTrue(e.getMessage().startsWith("Bad sheet name quote escaping: "));
+ }
+ }
+
+ @Test(expected=IllegalArgumentException.class)
+ public void negativeRow() {
+ new CellReference("sheet", -2, 0, false, false);
+ }
+ @Test(expected=IllegalArgumentException.class)
+ public void negativeColumn() {
+ new CellReference("sheet", 0, -2, false, false);
+ }
+
+ @Test(expected=IllegalArgumentException.class)
+ public void classifyEmptyStringCellReference() {
+ CellReference.classifyCellReference("", SpreadsheetVersion.EXCEL2007);
+ }
+ @Test(expected=IllegalArgumentException.class)
+ public void classifyInvalidFirstCharCellReference() {
+ CellReference.classifyCellReference("!A5", SpreadsheetVersion.EXCEL2007);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org