Merge pull request #165 from pjfanning/fixWrongTypeForStringFormula

Fix wrong type for string formula

src/main/java/com/monitorjbl/xlsx/impl/StreamingCell.java

@@ -40,24 +40,16 @@ public Object getContent() {
   private String numericFormat;
   private Short numericFormatIndex;
   private String type;
-  private String cachedFormulaResultType;
   private Row row;
   private CellStyle cellStyle;
+  private boolean formulaType;
 
   public StreamingCell(int columnIndex, int rowIndex, boolean use1904Dates) {
     this.columnIndex = columnIndex;
     this.rowIndex = rowIndex;
     this.use1904Dates = use1904Dates;
   }
 
-  String getRawCachedFormulaResultType() {
-    return cachedFormulaResultType;
-  }
-
-  boolean supportsSupplierOverride() {
-    return "n".equals(cachedFormulaResultType);
-  }
-
   public void setContentSupplier(Supplier contentsSupplier) {
     this.contentsSupplier = contentsSupplier;
   }
@@ -91,13 +83,17 @@ public String getType() {
   }
 
   public void setType(String type) {
-    if("str".equals(type)) {
-      // this is a formula cell, cache the value's type
-      cachedFormulaResultType = this.type;
-    }
     this.type = type;
   }
 
+  public boolean isFormulaType() {
+    return formulaType;
+  }
+
+  public void setFormulaType(boolean formulaType) {
+    this.formulaType = formulaType;
+  }
+
   public void setRow(Row row) {
     this.row = row;
   }
@@ -143,18 +139,17 @@ public Row getRow() {
   /**
    * Return the cell type.
    *
-   * Will return {@link CellType} in version 4.0 of POI.
-   * For forwards compatibility, do not hard-code cell type literals in your code.
-   *
    * @return the cell type
    */
   @Override
   public CellType getCellType() {
-    if(contentsSupplier.getContent() == null || type == null) {
+    if(formulaType) {
+      return CellType.FORMULA;
+    } else if(contentsSupplier.getContent() == null || type == null) {
       return CellType.BLANK;
     } else if("n".equals(type)) {
       return CellType.NUMERIC;
-    } else if("s".equals(type) || "inlineStr".equals(type)) {
+    } else if("s".equals(type) || "inlineStr".equals(type) || "str".equals(type)) {
       return CellType.STRING;
     } else if("str".equals(type)) {
       return CellType.FORMULA;
@@ -174,12 +169,13 @@ public CellType getCellType() {
    * Will be renamed to <code>getCellType()</code> when we make the CellType enum transition in POI 4.0. See bug 59791.
    */
   @Override
+  @Deprecated
   public CellType getCellTypeEnum() {
     return getCellType();
   }
 
   /**
-   * Get the value of the cell as a string. For numeric cells we throw an exception.
+   * Get the value of the cell as a string.
    * For blank cells we return an empty string.
    *
    * @return the value of the cell as a string
@@ -188,7 +184,7 @@ public CellType getCellTypeEnum() {
   public String getStringCellValue() {
     Object c = contentsSupplier.getContent();
 
-    return c == null ? "" : (String) c;
+    return c == null ? "" : c.toString();
   }
 
   /**
@@ -278,7 +274,7 @@ public CellStyle getCellStyle() {
    */
   @Override
   public String getCellFormula() {
-    if (type == null || !"str".equals(type))
+    if (!formulaType)
       throw new IllegalStateException("This cell does not have a formula");
     return formula;
   }
@@ -291,28 +287,26 @@ public String getCellFormula() {
    */
   @Override
   public CellType getCachedFormulaResultType() {
-    if (type != null && "str".equals(type)) {
-      if(contentsSupplier.getContent() == null || cachedFormulaResultType == null) {
+    if (formulaType) {
+      if(contentsSupplier.getContent() == null || type == null) {
         return CellType.BLANK;
-      } else if("n".equals(cachedFormulaResultType)) {
+      } else if("n".equals(type)) {
         return CellType.NUMERIC;
-      } else if("s".equals(cachedFormulaResultType) || "inlineStr".equals(cachedFormulaResultType)) {
+      } else if("s".equals(type) || "inlineStr".equals(type) || "str".equals(type)) {
         return CellType.STRING;
-      } else if("str".equals(cachedFormulaResultType)) {
-        return CellType.FORMULA;
-      } else if("b".equals(cachedFormulaResultType)) {
+      } else if("b".equals(type)) {
         return CellType.BOOLEAN;
-      } else if("e".equals(cachedFormulaResultType)) {
+      } else if("e".equals(type)) {
         return CellType.ERROR;
       } else {
-        throw new UnsupportedOperationException("Unsupported cell type '" + cachedFormulaResultType + "'");
+        throw new UnsupportedOperationException("Unsupported cell type '" + type + "'");
       }
-    }
-    else  {
+    } else  {
       throw new IllegalStateException("Only formula cells have cached results");
     }
   }
 
+  @Deprecated
   @Override
   public CellType getCachedFormulaResultTypeEnum() {
     return getCachedFormulaResultType();
@@ -394,7 +388,7 @@ public void setCellFormula(String formula) throws FormulaParseException {
    */
   @Override
   public XSSFRichTextString getRichStringCellValue() {
-    CellType cellType = getCellTypeEnum();
+    CellType cellType = getCellType();
     XSSFRichTextString rt;
     switch (cellType) {
       case BLANK:
@@ -512,4 +506,4 @@ public CellRangeAddress getArrayFormulaRange() {
   public boolean isPartOfArrayFormulaGroup() {
     throw new NotSupportedException();
   }
-}
+}

src/main/java/com/monitorjbl/xlsx/impl/StreamingSheetReader.java

@@ -190,7 +190,7 @@ && isSpreadsheetTag(event.asStartElement().getName())) {
         }
       } else if("f".equals(tagLocalName)) {
         if (currentCell != null) {
-          currentCell.setType("str");
+          currentCell.setFormulaType(true);
         }
       }
 
@@ -319,6 +319,7 @@ private Supplier getFormatterForType(String type) {
         }
         return new StringSupplier(lastContents);
       case "inlineStr":   //inline string (not in sst)
+      case "str":
         return new StringSupplier(new XSSFRichTextString(lastContents).toString());
       case "e":           //error type
         return new StringSupplier("ERROR:  " + lastContents);
@@ -349,10 +350,6 @@ public Object getContent() {
         } else {
           return new StringSupplier(lastContents);
         }
-      case "str":         //formula type
-        if (currentCell.supportsSupplierOverride()) {
-          return getFormatterForType(currentCell.getRawCachedFormulaResultType());
-        }
       default:
         return new StringSupplier(lastContents);
     }

src/test/java/com/monitorjbl/xlsx/StreamingReaderTest.java

@@ -7,6 +7,7 @@
 import org.apache.poi.ss.usermodel.CellType;
 import org.apache.poi.ss.usermodel.DateUtil;
 import org.apache.poi.ss.usermodel.Row;
+import org.apache.poi.ss.usermodel.Sheet;
 import org.apache.poi.ss.usermodel.Workbook;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -693,4 +694,38 @@ public void testForumulaOutsideCellIgnored() throws Exception {
       assertThat(cell.getCellTypeEnum(), is(CellType.NUMERIC));
     }
   }
+
+  @Test
+  public void testFormulaWithDifferentTypes() throws Exception {
+    try(
+      InputStream is = new FileInputStream(new File("src/test/resources/formula_test.xlsx"));
+      Workbook wb = StreamingReader.builder().open(is)
+    ) {
+      Sheet sheet = wb.getSheetAt(0);
+      Iterator<Row> rowIterator = sheet.rowIterator();
+
+      Row next = rowIterator.next();
+      Cell cell = next.getCell(0);
+
+      assertThat(cell.getCellTypeEnum(), is(CellType.STRING));
+
+      next = rowIterator.next();
+      cell = next.getCell(0);
+
+      assertThat(cell.getCellTypeEnum(), is(CellType.FORMULA));
+      assertThat(cell.getCachedFormulaResultTypeEnum(), is(CellType.STRING));
+
+      next = rowIterator.next();
+      cell = next.getCell(0);
+
+      assertThat(cell.getCellTypeEnum(), is(CellType.FORMULA));
+      assertThat(cell.getCachedFormulaResultTypeEnum(), is(CellType.BOOLEAN));
+
+      next = rowIterator.next();
+      cell = next.getCell(0);
+
+      assertThat(cell.getCellTypeEnum(), is(CellType.FORMULA));
+      assertThat(cell.getCachedFormulaResultTypeEnum(), is(CellType.NUMERIC));
+    }
+  }
 }