Fix for bug 44708. XSSFCell.getCellType() now returns CELL_TYPE_BLANK for numeric cells with no value.

git-svn-id: https://svn.apache.org/repos/asf/poi/branches/ooxml@645298 13f79535-47bb-0310-9956-ffa450edef68

build.xml

@@ -236,6 +236,7 @@ under the License.
         <path refid="ooxml.classpath"/>
         <pathelement location="${ooxml.output.dir}"/>
         <pathelement location="${ooxml.output.test.dir}"/>
+        <pathelement location="${main.output.test.dir}"/> <!-- ooxml tests use some utilities from main tests -->
         <pathelement location="${junit.jar1.dir}"/>
     </path>
 
@@ -790,7 +791,7 @@ under the License.
             <batchtest todir="${ooxml.reports.test}">
                 <fileset dir="${ooxml.src.test}">
                     <include name="**/Test*.java"/>
-                    <exclude name="**/AllTests.java"/>
+                    <exclude name="**/All*Tests.java"/>
                 </fileset>
             </batchtest>
         </junit>

src/ooxml/interfaces-jdk15/org/apache/poi/ss/usermodel/Cell.java

@@ -117,11 +117,12 @@ public interface Cell {
     void setCellType(int cellType);
 
     /**
-     * get the cells type (numeric, formula or string)
+     * @return the cell's type (e.g. numeric, formula or string)
      * @see #CELL_TYPE_STRING
      * @see #CELL_TYPE_NUMERIC
      * @see #CELL_TYPE_FORMULA
      * @see #CELL_TYPE_BOOLEAN
+     * @see #CELL_TYPE_BLANK
      * @see #CELL_TYPE_ERROR
      */
 

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java

@@ -35,8 +35,10 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCell;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCellFormula;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.STCellType;
 
-
+/**
-public class XSSFCell implements Cell {
+ *
+ */
+public final class XSSFCell implements Cell {
 
     private static final String FALSE_AS_STRING = "0";
     private static final String TRUE_AS_STRING  = "1";
@@ -118,6 +120,14 @@ public class XSSFCell implements Cell {
         case STCellType.INT_B:
             return CELL_TYPE_BOOLEAN;
         case STCellType.INT_N:
+            if(!cell.isSetV()) {
+                // ooxml does have a separate cell type of 'blank'.  A blank cell gets encoded as
+                // (either not present or) a numeric cell with no value set.
+                // The formula evaluator (and perhaps other clients of this interface) needs to
+                // distinguish blank values which sometimes get translated into zero and sometimes
+                // empty string, depending on context
+                return CELL_TYPE_BLANK;
+            }
             return CELL_TYPE_NUMERIC;
         case STCellType.INT_E:
             return CELL_TYPE_ERROR;
@@ -196,8 +206,25 @@ public class XSSFCell implements Cell {
         if (this.cell.isSetV()) {
             return Double.parseDouble(this.cell.getV());
         }
+        // else - cell is blank.
+
+        // TODO - behaviour in the case of blank cells.
+        // Revise spec, choose best alternative below, and comment why.
+        if (true) {
+            // returning NaN from a blank cell seems wrong
+            // there are a few junits which assert this behaviour, though.
             return Double.NaN;
         }
+        if (true) {
+            // zero is probably a more reasonable value.
+            return 0.0;
+        } else {
+            // or perhaps disallow reading value from blank cell.
+            throw new RuntimeException("You cannot get a numeric value from a blank cell");
+        }
+        // Note - it would be nice if the behaviour is consistent with getRichStringCellValue
+        // (i.e. whether to return empty string or throw exception).
+    }
 
     public RichTextString getRichStringCellValue() {
         if(this.cell.getT() == STCellType.INLINE_STR) {

src/ooxml/testcases/org/apache/poi/xssf/AllXSSFTests.java

@@ -0,0 +1,54 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.xssf;
+
+import junit.framework.Test;
+import junit.framework.TestSuite;
+
+import org.apache.poi.xssf.eventusermodel.TestXSSFReader;
+import org.apache.poi.xssf.extractor.TestXSSFExcelExtractor;
+import org.apache.poi.xssf.io.TestLoadSaveXSSF;
+import org.apache.poi.xssf.model.TestCommentsTable;
+import org.apache.poi.xssf.model.TestStylesTable;
+import org.apache.poi.xssf.usermodel.AllXSSFUsermodelTests;
+import org.apache.poi.xssf.util.TestCTColComparator;
+import org.apache.poi.xssf.util.TestCellReference;
+import org.apache.poi.xssf.util.TestNumericRanges;
+
+/**
+ * Collects all tests for <tt>org.apache.poi.xssf</tt> and sub-packages.
+ * 
+ * @author Josh Micich
+ */
+public final class AllXSSFTests {
+
+	public static Test suite() {
+		TestSuite result = new TestSuite(AllXSSFTests.class.getName());
+		result.addTest(AllXSSFUsermodelTests.suite());
+		
+		result.addTestSuite(TestXSSFReader.class);
+		result.addTestSuite(TestXSSFExcelExtractor.class);
+		result.addTestSuite(TestLoadSaveXSSF.class);
+		result.addTestSuite(TestCommentsTable.class);
+		result.addTestSuite(TestStylesTable.class);
+		result.addTestSuite(TestCellReference.class);
+		result.addTestSuite(TestCTColComparator.class);
+		result.addTestSuite(TestNumericRanges.class);
+		return result;
+	}
+}

src/ooxml/testcases/org/apache/poi/xssf/usermodel/AllXSSFUsermodelTests.java

@@ -0,0 +1,56 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.xssf.usermodel;
+
+import junit.framework.Test;
+import junit.framework.TestSuite;
+
+import org.apache.poi.xssf.usermodel.extensions.TestXSSFBorder;
+import org.apache.poi.xssf.usermodel.extensions.TestXSSFCellFill;
+import org.apache.poi.xssf.usermodel.extensions.TestXSSFSheetComments;
+import org.apache.poi.xssf.usermodel.helpers.TestColumnHelper;
+import org.apache.poi.xssf.usermodel.helpers.TestHeaderFooterHelper;
+
+/**
+ * Collects all tests for <tt>org.apache.poi.xssf.usermodel</tt> and sub-packages.
+ * 
+ * @author Josh Micich
+ */
+public final class AllXSSFUsermodelTests {
+
+	public static Test suite() {
+		TestSuite result = new TestSuite(AllXSSFUsermodelTests.class.getName());
+		result.addTestSuite(TestXSSFBorder.class);
+		result.addTestSuite(TestXSSFCellFill.class);
+		result.addTestSuite(TestXSSFHeaderFooter.class);
+		result.addTestSuite(TestXSSFSheetComments.class);
+		result.addTestSuite(TestColumnHelper.class);
+		result.addTestSuite(TestHeaderFooterHelper.class);
+		result.addTestSuite(TestFormulaEvaluatorOnXSSF.class);
+		result.addTestSuite(TestXSSFCell.class);
+		result.addTestSuite(TestXSSFCellStyle.class);
+		result.addTestSuite(TestXSSFComment.class);
+		result.addTestSuite(TestXSSFDialogSheet.class);
+		result.addTestSuite(TestXSSFFormulaEvaluation.class);
+		result.addTestSuite(TestXSSFHeaderFooter.class);
+		result.addTestSuite(TestXSSFRow.class);
+		result.addTestSuite(TestXSSFSheet.class);
+		result.addTestSuite(TestXSSFWorkbook.class);
+		return result;
+	}
+}

src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestFormulaEvaluatorOnXSSF.java

@@ -41,7 +41,6 @@ import org.openxml4j.opc.Package;
  * Periodically, you should open FormulaEvalTestData.xls in
  *  Excel 2007, and re-save it as FormulaEvalTestData_Copy.xlsx
  *  
- * Currently disabled, as doesn't work
  */
 public final class TestFormulaEvaluatorOnXSSF extends TestCase {
 	
@@ -178,7 +177,7 @@ public final class TestFormulaEvaluatorOnXSSF extends TestCase {
 	 * Disabled for now, as many things seem to break
 	 *  for XSSF, which is a shame
 	 */
-	public void DISABLEDtestFunctionsFromTestSpreadsheet() {
+	public void testFunctionsFromTestSpreadsheet() {
 		
 		processFunctionGroup(SS.START_OPERATORS_ROW_INDEX, null);
 		processFunctionGroup(SS.START_FUNCTIONS_ROW_INDEX, null);
@@ -261,8 +260,19 @@ public final class TestFormulaEvaluatorOnXSSF extends TestCase {
 			if (c == null || c.getCellType() != Cell.CELL_TYPE_FORMULA) {
 				continue;
 			}
+			if(isIgnoredFormulaTestCase(c.getCellFormula())) {
+				continue;
+			}
 
-			FormulaEvaluator.CellValue actualValue = evaluator.evaluate(c);
+			FormulaEvaluator.CellValue actualValue;
+			try {
+				actualValue = evaluator.evaluate(c);
+			} catch (RuntimeException e) {
+				_evaluationFailureCount ++;
+				printShortStackTrace(System.err, e);
+				result = Result.SOME_EVALUATIONS_FAILED;
+				continue;
+			}
 
 			Cell expectedValueCell = getExpectedValueCell(expectedValuesRow, colnum);
 			try {
@@ -281,10 +291,29 @@ public final class TestFormulaEvaluatorOnXSSF extends TestCase {
  		return result;
 	}
 
+	/*
+	 * TODO - these are all formulas which currently (Apr-2008) break on ooxml 
+	 */
+	private static boolean isIgnoredFormulaTestCase(String cellFormula) {
+		if ("COLUMN(1:2)".equals(cellFormula) || "ROW(2:3)".equals(cellFormula)) {
+			// full row ranges are not parsed properly yet.
+			// These cases currently work in svn trunk because of another bug which causes the 
+			// formula to get rendered as COLUMN($A$1:$IV$2) or ROW($A$2:$IV$3) 
+			return true;
+		}
+		if ("ISREF(currentcell())".equals(cellFormula)) {
+			// currently throws NPE because unknown function "currentcell" causes name lookup 
+			// Name lookup requires some equivalent object of the Workbook within xSSFWorkbook.
+			return true;
+		}
+		return false;
+	}
+
+
 	/**
 	 * Useful to keep output concise when expecting many failures to be reported by this test case
 	 */
-	private static void printShortStackTrace(PrintStream ps, AssertionFailedError e) {
+	private static void printShortStackTrace(PrintStream ps, Throwable e) {
 		StackTraceElement[] stes = e.getStackTrace();
 		
 		int startIx = 0;

src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFRow.java

@@ -24,8 +24,10 @@ import junit.framework.TestCase;
 import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.xssf.usermodel.TestXSSFCell.DummySharedStringSource;
 
-
+/**
-public class TestXSSFRow extends TestCase {
+ * Tests for XSSFRow
+ */
+public final class TestXSSFRow extends TestCase {
 
     /**
      * Test adding cells to a row in various places and see if we can find them again.
@@ -84,22 +86,24 @@ public class TestXSSFRow extends TestCase {
         assertEquals(Cell.CELL_TYPE_STRING, cell5.getCellType());
     }
 
-    public void testGetCell() throws Exception {
+    public void testGetCell() {
         XSSFRow row = getSampleRow();
 
         assertNotNull(row.getCell((short) 2));
         assertNotNull(row.getCell((short) 3));
         assertNotNull(row.getCell((short) 4));
-        assertEquals(Cell.CELL_TYPE_NUMERIC, row.getCell((short) 3).getCellType());
+        // cell3 may have been created as CELL_TYPE_NUMERIC, but since there is no numeric
+        // value set yet, its cell type is classified as 'blank'
+        assertEquals(Cell.CELL_TYPE_BLANK, row.getCell((short) 3).getCellType());
         assertNull(row.getCell((short) 5));
     }
 
-    public void testGetPhysicalNumberOfCells() throws Exception {
+    public void testGetPhysicalNumberOfCells() {
         XSSFRow row = getSampleRow();
         assertEquals(7, row.getPhysicalNumberOfCells());
     }
 
-    public void testGetFirstCellNum() throws Exception {
+    public void testGetFirstCellNum() {
         // Test a row with some cells
         XSSFRow row = getSampleRow();
         assertFalse(row.getFirstCellNum() == (short) 0);
@@ -115,7 +119,7 @@ public class TestXSSFRow extends TestCase {
         assertEquals(-1, emptyRow.getFirstCellNum());
     }
 
-    public void testLastCellNum() throws Exception {
+    public void testLastCellNum() {
         XSSFRow row = getSampleRow();
         assertEquals(100, row.getLastCellNum());
 
@@ -124,7 +128,7 @@ public class TestXSSFRow extends TestCase {
         assertFalse(row.getLastCellNum() == (short) 100);
     }
 
-    public void testRemoveCell() throws Exception {
+    public void testRemoveCell() {
         XSSFRow row = getSampleRow();
 
         // Test removing the first cell
@@ -139,10 +143,9 @@ public class TestXSSFRow extends TestCase {
         // Test removing the last cell
         Cell lastCell = row.getCell((short) 100);
         row.removeCell(lastCell);
-    	
     }
 
-    public void testGetSetHeight() throws Exception {
+    public void testGetSetHeight() {
         XSSFRow row = getSampleRow();
         // I assume that "ht" attribute value is in 'points', please verify that
         // Test that no rowHeight is set
@@ -150,9 +153,9 @@ public class TestXSSFRow extends TestCase {
         // Set a rowHeight in twips (1/20th of a point) and test the new value
         row.setHeight((short) 240);
         assertEquals((short) 240, row.getHeight());
-        assertEquals((float) 12, row.getHeightInPoints());
+        assertEquals(12F, row.getHeightInPoints());
         // Set a new rowHeight in points and test the new value
-        row.setHeightInPoints((float) 13);
+        row.setHeightInPoints(13F);
         assertEquals((float) 13, row.getHeightInPoints());
         assertEquals((short) 260, row.getHeight());
     }
@@ -168,7 +171,7 @@ public class TestXSSFRow extends TestCase {
      * Method that returns a row with some sample cells
      * @return row
      */
-    public XSSFRow getSampleRow() {
+    private static XSSFRow getSampleRow() {
         XSSFRow row = new XSSFRow(createParentObjects());
         row.createCell((short) 2);
         row.createCell((short) 3, Cell.CELL_TYPE_NUMERIC);
@@ -180,7 +183,7 @@ public class TestXSSFRow extends TestCase {
         return row;
     }
 
-    private XSSFSheet createParentObjects() {
+    private static XSSFSheet createParentObjects() {
         XSSFWorkbook wb = new XSSFWorkbook();
         wb.setSharedStringSource(new DummySharedStringSource());
         return new XSSFSheet(wb);