Bugs 54228,53672 - Fixed XSSF to read cells with missing R attribute

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

5 files changed, 109 insertions, 4 deletions

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index 2374823a42..35f3cbef91 100644
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -34,6 +34,7 @@
 
     <changes>
         <release version="4.0-beta1" date="2013-??-??">
+          <action dev="poi-developers" type="fix">54228,53672 - Fixed XSSF to read cells with missing R attribute</action>
           <action dev="poi-developers" type="fix">54206 - Ensure that shared formuals are updated when shifting rows in a spreadsheet</action>
           <action dev="poi-developers" type="fix">Synchronize table headers with parent sheet in XSSF</action>
           <action dev="poi-developers" type="fix">54210 - Fixed rendering text in flipped shapes in PPT2PNG and PPTX2PNG</action>
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 58b3c561d7..ea3531eb52 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java
@@ -104,6 +104,11 @@ public final class XSSFCell implements Cell {
         _row = row;
         if (cell.getR() != null) {
             _cellNum = new CellReference(cell.getR()).getCol();
+        } else {
+            int prevNum = row.getLastCellNum();
+            if(prevNum != -1){
+                _cellNum = row.getCell(prevNum-1).getColumnIndex() + 1;
+            }
         }
         _sharedStringSource = row.getSheet().getWorkbook().getSharedStringSource();
         _stylesSource = row.getSheet().getWorkbook().getStylesSource();
@@ -468,7 +473,11 @@ public final class XSSFCell implements Cell {
      * @return A1 style reference to the location of this cell
      */
     public String getReference() {
-        return _cell.getR();
+        String ref = _cell.getR();
+        if(ref == null) {
+            return new CellReference(this).formatAsString();
+        }
+        return ref;
     }
 
     /**
@@ -685,7 +694,7 @@ public final class XSSFCell implements Cell {
      * Sets this cell as the active cell for the worksheet.
      */
     public void setAsActiveCell() {
-        getSheet().setActiveCell(_cell.getR());
+        getSheet().setActiveCell(getReference());
     }
 
     /**
@@ -889,7 +898,7 @@ public final class XSSFCell implements Cell {
     public void removeCellComment() {
         XSSFComment comment = getCellComment();
         if(comment != null){
-            String ref = _cell.getR();
+            String ref = getReference();
             XSSFSheet sh = getSheet();
             sh.getCommentsTable(false).removeComment(ref);
             sh.getVMLDrawing(false).removeCommentShape(getRowIndex(), getColumnIndex());
@@ -1009,7 +1018,7 @@ public final class XSSFCell implements Cell {
     public CellRangeAddress getArrayFormulaRange() {
         XSSFCell cell = getSheet().getFirstCellInArrayFormula(this);
         if (cell == null) {
-            throw new IllegalStateException("Cell " + _cell.getR()
+            throw new IllegalStateException("Cell " + getReference()
                     + " is not part of an array formula.");
         }
         String formulaRef = cell._cell.getF().getRef();
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 0691363566..44583fdf66 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCell.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCell.java
@@ -23,6 +23,9 @@ import org.apache.poi.xssf.model.SharedStringsTable;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.STCellType;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCell;
 
+import java.io.FileOutputStream;
+import java.io.IOException;
+
 /**
  * @author Yegor Kozlov
  */
@@ -182,4 +185,96 @@ public final class TestXSSFCell extends BaseTestCell {
         wb.getCreationHelper().createFormulaEvaluator().evaluateFormulaCell(cell);
         assertEquals(36, cell.getErrorCellValue());
     }
+
+    public void testMissingRAttribute() {
+        XSSFWorkbook wb = new XSSFWorkbook();
+        XSSFSheet sheet = wb.createSheet();
+        XSSFRow row = sheet.createRow(0);
+        XSSFCell a1 = row.createCell(0);
+        a1.setCellValue("A1");
+        XSSFCell a2 = row.createCell(1);
+        a2.setCellValue("B1");
+        XSSFCell a4 = row.createCell(4);
+        a4.setCellValue("E1");
+        XSSFCell a6 = row.createCell(5);
+        a6.setCellValue("F1");
+
+        assertCellsWithMissingR(row);
+
+        a2.getCTCell().unsetR();
+        a6.getCTCell().unsetR();
+
+        assertCellsWithMissingR(row);
+
+        wb = (XSSFWorkbook)_testDataProvider.writeOutAndReadBack(wb);
+        row = wb.getSheetAt(0).getRow(0);
+        assertCellsWithMissingR(row);
+    }
+
+    private void assertCellsWithMissingR(XSSFRow row){
+        XSSFCell a1 = row.getCell(0);
+        assertNotNull(a1);
+        XSSFCell a2 = row.getCell(1);
+        assertNotNull(a2);
+        XSSFCell a5 = row.getCell(4);
+        assertNotNull(a5);
+        XSSFCell a6 = row.getCell(5);
+        assertNotNull(a6);
+
+        assertEquals(6, row.getLastCellNum());
+        assertEquals(4, row.getPhysicalNumberOfCells());
+
+        assertEquals("A1", a1.getStringCellValue());
+        assertEquals("B1", a2.getStringCellValue());
+        assertEquals("E1", a5.getStringCellValue());
+        assertEquals("F1", a6.getStringCellValue());
+
+        // even if R attribute is not set,
+        // POI is able to re-construct it from column and row indexes
+        assertEquals("A1", a1.getReference());
+        assertEquals("B1", a2.getReference());
+        assertEquals("E1", a5.getReference());
+        assertEquals("F1", a6.getReference());
+    }
+
+    public void testMissingRAttributeBug54288() {
+        // workbook with cells missing the R attribute
+        XSSFWorkbook wb = (XSSFWorkbook)_testDataProvider.openSampleWorkbook("54288.xlsx");
+        // same workbook re-saved in Excel 2010, the R attribute is updated for every cell with the right value.
+        XSSFWorkbook wbRef = (XSSFWorkbook)_testDataProvider.openSampleWorkbook("54288-ref.xlsx");
+
+        XSSFSheet sheet = wb.getSheetAt(0);
+        XSSFSheet sheetRef = wbRef.getSheetAt(0);
+        assertEquals(sheetRef.getPhysicalNumberOfRows(), sheet.getPhysicalNumberOfRows());
+
+        // Test idea: iterate over cells in the reference worksheet, they all have the R attribute set.
+        // For each cell from the reference sheet find the corresponding cell in the problematic file (with missing R)
+        // and assert that POI reads them equally:
+        DataFormatter formater = new DataFormatter();
+        for(Row r : sheetRef){
+            XSSFRow rowRef = (XSSFRow)r;
+            XSSFRow row = sheet.getRow(rowRef.getRowNum());
+
+            assertEquals("number of cells in row["+row.getRowNum()+"]",
+                    rowRef.getPhysicalNumberOfCells(), row.getPhysicalNumberOfCells());
+
+            for(Cell c :  rowRef){
+                XSSFCell cellRef = (XSSFCell)c;
+                XSSFCell cell = row.getCell(cellRef.getColumnIndex());
+
+                assertEquals(cellRef.getColumnIndex(), cell.getColumnIndex());
+                assertEquals(cellRef.getReference(), cell.getReference());
+
+                if(!cell.getCTCell().isSetR()){
+                    assertTrue("R must e set in cellRef", cellRef.getCTCell().isSetR());
+
+                    String valRef = formater.formatCellValue(cellRef);
+                    String val = formater.formatCellValue(cell);
+                    assertEquals(valRef, val);
+                }
+
+            }
+        }
+    }
+
 }
diff --git a/test-data/spreadsheet/54288-ref.xlsx b/test-data/spreadsheet/54288-ref.xlsx
new file mode 100644
index 0000000000..eb2f8396cf
--- /dev/null
+++ b/test-data/spreadsheet/54288-ref.xlsx
diff --git a/test-data/spreadsheet/54288.xlsx b/test-data/spreadsheet/54288.xlsx
new file mode 100644
index 0000000000..fed0b4bed7
--- /dev/null
+++ b/test-data/spreadsheet/54288.xlsx