Fix a regression test failure, toString() should never return null

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

5 files changed, 100 insertions, 91 deletions

diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRichTextString.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRichTextString.java
index fbc06a9709..225b9eb860 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRichTextString.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFRichTextString.java
@@ -307,11 +307,15 @@ public class XSSFRichTextString implements RichTextString {
 
     /**
      * Returns the plain string representation.
+     *
+     * @return The string representation of this RichText string, null if
+     *          there is no data at all
      */
     public String getString() {
         if(st.sizeOfRArray() == 0) {
             return utfDecode(st.getT());
         }
+
         StringBuilder buf = new StringBuilder();
         //noinspection deprecation - for performance reasons!
         for(CTRElt r : st.getRArray()){
@@ -333,9 +337,16 @@ public class XSSFRichTextString implements RichTextString {
 
     /**
      * Returns the plain string representation.
+     *
+     * @return The string representation of this RichText string, never null
      */
     public String toString() {
-        return getString();
+        String str = getString();
+        if(str == null) {
+            return "";
+        }
+
+        return str;
     }
 
     /**
@@ -495,7 +506,7 @@ public class XSSFRichTextString implements RichTextString {
      * See section 3.18.9 in the OOXML spec.
      *
      * @param   value the string to decode
-     * @return  the decoded string
+     * @return  the decoded string or null if the input string is null
      */
     static String utfDecode(String value) {
         if(value == null || !value.contains("_x")) {
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSimpleShape.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSimpleShape.java
index b8ba72522c..92a173e743 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSimpleShape.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSimpleShape.java
@@ -177,8 +177,6 @@ public class XSSFSimpleShape extends XSSFShape implements Iterable<XSSFTextParag
         StringBuilder out = new StringBuilder();
         List<Integer> levelCount = new ArrayList<>(MAX_LEVELS); // maximum 9
                                                                 // levels
-        XSSFTextParagraph p = null;
-
         // initialise the levelCount array - this maintains a record of the
         // numbering to be used at each level
         for (int k = 0; k < MAX_LEVELS; k++) {
@@ -189,7 +187,7 @@ public class XSSFSimpleShape extends XSSFShape implements Iterable<XSSFTextParag
             if (out.length() > 0) {
                 out.append('\n');
             }
-            p = _paragraphs.get(i);
+            XSSFTextParagraph p = _paragraphs.get(i);
 
             if (p.isBullet() && p.getText().length() > 0) {
 
@@ -223,12 +221,7 @@ public class XSSFSimpleShape extends XSSFShape implements Iterable<XSSFTextParag
      *
      */
     private int processAutoNumGroup(int index, int level, List<Integer> levelCount, StringBuilder out) {
-        XSSFTextParagraph p = null;
-        XSSFTextParagraph nextp = null;
-        ListAutoNumber scheme, nextScheme;
-        int startAt, nextStartAt;
-
-        p = _paragraphs.get(index);
+        XSSFTextParagraph p = _paragraphs.get(index);
 
         // The rules for generating the auto numbers are as follows. If the
         // following paragraph is also
@@ -242,8 +235,8 @@ public class XSSFSimpleShape extends XSSFShape implements Iterable<XSSFTextParag
 
         // first auto-number paragraph so initialise to 1 or the bullets startAt
         // if present
-        startAt = p.getBulletAutoNumberStart();
-        scheme = p.getBulletAutoNumberScheme();
+        int startAt = p.getBulletAutoNumberStart();
+        ListAutoNumber scheme = p.getBulletAutoNumberScheme();
         if (levelCount.get(level) == 0) {
             levelCount.set(level, startAt == 0 ? 1 : startAt);
         }
@@ -256,7 +249,7 @@ public class XSSFSimpleShape extends XSSFShape implements Iterable<XSSFTextParag
             out.append(p.getText());
         }
         while (true) {
-            nextp = (index + 1) == _paragraphs.size() ? null : _paragraphs.get(index + 1);
+            XSSFTextParagraph nextp = (index + 1) == _paragraphs.size() ? null : _paragraphs.get(index + 1);
             if (nextp == null) {
                 break; // out of paragraphs
             }
@@ -273,8 +266,8 @@ public class XSSFSimpleShape extends XSSFShape implements Iterable<XSSFTextParag
             } else if (nextp.getLevel() < level) {
                 break; // changed level
             }
-            nextScheme = nextp.getBulletAutoNumberScheme();
-            nextStartAt = nextp.getBulletAutoNumberStart();
+            ListAutoNumber nextScheme = nextp.getBulletAutoNumberScheme();
+            int nextStartAt = nextp.getBulletAutoNumberStart();
 
             if (nextScheme == scheme && nextStartAt == startAt) {
                 // bullet is valid, so increment i
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 993e2c9b21..c7bc49d546 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCell.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCell.java
@@ -133,13 +133,20 @@ public final class TestXSSFCell extends BaseTestXCell {
         assertNull(str.getString());
         cell_0.setCellValue(str);
         assertEquals(0, sst.getCount());
-        assertEquals(CellType.BLANK, cell_0.getCellType());
+        assertEquals("Having: " + cell_0, CellType.BLANK, cell_0.getCellType());
 
         //case 2. cell.setCellValue((String)null);
         Cell cell_1 = row.createCell(1);
         cell_1.setCellValue((String)null);
         assertEquals(0, sst.getCount());
-        assertEquals(CellType.BLANK, cell_1.getCellType());
+        assertEquals("Having: " + cell_1, CellType.BLANK, cell_1.getCellType());
+
+        //case 3. cell.setCellValue((RichTextString)null);
+        Cell cell_2 = row.createCell(2);
+        cell_2.setCellValue((RichTextString) null);
+        assertEquals(0, sst.getCount());
+        assertEquals("Having: " + cell_2, CellType.BLANK, cell_2.getCellType());
+
         wb.close();
     }
 
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFRichTextString.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFRichTextString.java
index a0129943df..0827b5b24f 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFRichTextString.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFRichTextString.java
@@ -41,19 +41,19 @@ public final class TestXSSFRichTextString extends TestCase {
     public void testCreate() {
         XSSFRichTextString rt = new XSSFRichTextString("Apache POI");
         assertEquals("Apache POI", rt.getString());
-        assertEquals(false, rt.hasFormatting());
+        assertFalse(rt.hasFormatting());
 
         CTRst st = rt.getCTRst();
         assertTrue(st.isSetT());
         assertEquals("Apache POI", st.getT());
-        assertEquals(false, rt.hasFormatting());
+        assertFalse(rt.hasFormatting());
 
         rt.append(" is cool stuff");
         assertEquals(2, st.sizeOfRArray());
         assertFalse(st.isSetT());
 
         assertEquals("Apache POI is cool stuff", rt.getString());
-        assertEquals(false, rt.hasFormatting());
+        assertFalse(rt.hasFormatting());
     }
 
     public void testEmpty() {
@@ -70,13 +70,13 @@ public final class TestXSSFRichTextString extends TestCase {
         rt.append("89");
 
         assertEquals("123456789", rt.getString());
-        assertEquals(false, rt.hasFormatting());
+        assertFalse(rt.hasFormatting());
 
         XSSFFont font1 = new XSSFFont();
         font1.setBold(true);
 
         rt.applyFont(2, 5, font1);
-        assertEquals(true, rt.hasFormatting());
+        assertTrue(rt.hasFormatting());
 
         assertEquals(4, rt.numFormattingRuns());
         assertEquals(0, rt.getIndexOfFormattingRun(0));
@@ -157,7 +157,7 @@ public final class TestXSSFRichTextString extends TestCase {
 
         XSSFRichTextString rt = new XSSFRichTextString("Apache POI");
         assertEquals("Apache POI", rt.getString());
-        assertEquals(false, rt.hasFormatting());
+        assertFalse(rt.hasFormatting());
 
         rt.clearFormatting();
 
@@ -165,20 +165,20 @@ public final class TestXSSFRichTextString extends TestCase {
         assertTrue(st.isSetT());
         assertEquals("Apache POI", rt.getString());
         assertEquals(0, rt.numFormattingRuns());
-        assertEquals(false, rt.hasFormatting());
+        assertFalse(rt.hasFormatting());
 
         XSSFFont font = new XSSFFont();
         font.setBold(true);
 
         rt.applyFont(7, 10, font);
         assertEquals(2, rt.numFormattingRuns());
-        assertEquals(true, rt.hasFormatting());
+        assertTrue(rt.hasFormatting());
         
         rt.clearFormatting();
         
         assertEquals("Apache POI", rt.getString());
         assertEquals(0, rt.numFormattingRuns());
-        assertEquals(false, rt.hasFormatting());
+        assertFalse(rt.hasFormatting());
     }
 
     public void testGetFonts() {
@@ -227,7 +227,7 @@ public final class TestXSSFRichTextString extends TestCase {
     /**
      * test that unicode representation_ xHHHH_ is properly processed
      */
-    public void testUtfDecode() throws IOException {
+    public void testUtfDecode() {
         CTRst st = CTRst.Factory.newInstance();
         st.setT("abc_x000D_2ef_x000D_");
         XSSFRichTextString rt = new XSSFRichTextString(st);
@@ -382,7 +382,7 @@ public final class TestXSSFRichTextString extends TestCase {
         assertEquals(" Software Foundation", str.getCTRst().getRArray(1).getT());
     }
 
-    public void testLineBreaks_bug48877() throws IOException{
+    public void testLineBreaks_bug48877() {
 
         XSSFFont font = new XSSFFont();
         font.setBold(true);
@@ -441,27 +441,28 @@ public final class TestXSSFRichTextString extends TestCase {
         assertEquals("<xml-fragment xml:space=\"preserve\">\n\n</xml-fragment>", t3.xmlText());
     }
 
-    public void testBug56511() {
-        XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("56511.xlsx");
-        for (Sheet sheet : wb) {
-            int lastRow = sheet.getLastRowNum();
-            for (int rowIdx = sheet.getFirstRowNum(); rowIdx <= lastRow; rowIdx++) {
-                Row row = sheet.getRow(rowIdx);
-                if(row != null) {
-                    int lastCell = row.getLastCellNum();
-    
-                    for (int cellIdx = row.getFirstCellNum(); cellIdx <= lastCell; cellIdx++) {
-    
-                        Cell cell = row.getCell(cellIdx);
-                        if (cell != null) {
-                            //System.out.println("row " + rowIdx + " column " + cellIdx + ": " + cell.getCellType() + ": " + cell.toString());
-                            
-                            XSSFRichTextString richText = (XSSFRichTextString) cell.getRichStringCellValue();
-                            int anzFormattingRuns = richText.numFormattingRuns();
-                            for (int run = 0; run < anzFormattingRuns; run++) {
-                                /*XSSFFont font =*/ richText.getFontOfFormattingRun(run);
-                                //System.out.println("  run " + run
-                                //        + " font " + (font == null ? "<null>" : font.getFontName()));
+    public void testBug56511() throws IOException {
+        try (XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("56511.xlsx")) {
+            for (Sheet sheet : wb) {
+                int lastRow = sheet.getLastRowNum();
+                for (int rowIdx = sheet.getFirstRowNum(); rowIdx <= lastRow; rowIdx++) {
+                    Row row = sheet.getRow(rowIdx);
+                    if (row != null) {
+                        int lastCell = row.getLastCellNum();
+
+                        for (int cellIdx = row.getFirstCellNum(); cellIdx <= lastCell; cellIdx++) {
+
+                            Cell cell = row.getCell(cellIdx);
+                            if (cell != null) {
+                                //System.out.println("row " + rowIdx + " column " + cellIdx + ": " + cell.getCellType() + ": " + cell.toString());
+
+                                XSSFRichTextString richText = (XSSFRichTextString) cell.getRichStringCellValue();
+                                int anzFormattingRuns = richText.numFormattingRuns();
+                                for (int run = 0; run < anzFormattingRuns; run++) {
+                                    /*XSSFFont font =*/ richText.getFontOfFormattingRun(run);
+                                    //System.out.println("  run " + run
+                                    //        + " font " + (font == null ? "<null>" : font.getFontName()));
+                                }
                             }
                         }
                     }
@@ -470,51 +471,51 @@ public final class TestXSSFRichTextString extends TestCase {
         }
     }
 
-    public void testBug56511_values() {
-        XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("56511.xlsx");
-        Sheet sheet = wb.getSheetAt(0);
-        Row row = sheet.getRow(0);
+    public void testBug56511_values() throws IOException {
+        try (XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("56511.xlsx")) {
+            Sheet sheet = wb.getSheetAt(0);
+            Row row = sheet.getRow(0);
 
-        // verify the values to ensure future changes keep the returned information equal 
-        XSSFRichTextString rt = (XSSFRichTextString) row.getCell(0).getRichStringCellValue();
-        assertEquals(0, rt.numFormattingRuns());
-        assertNull(rt.getFontOfFormattingRun(0));
-        assertEquals(-1, rt.getLengthOfFormattingRun(0));
-        
-        rt = (XSSFRichTextString) row.getCell(1).getRichStringCellValue();
-        assertEquals(0, row.getCell(1).getRichStringCellValue().numFormattingRuns());
-        assertNull(rt.getFontOfFormattingRun(1));
-        assertEquals(-1, rt.getLengthOfFormattingRun(1));
-               
-        rt = (XSSFRichTextString) row.getCell(2).getRichStringCellValue();
-        assertEquals(2, rt.numFormattingRuns());
-        assertNotNull(rt.getFontOfFormattingRun(0));
-        assertEquals(4, rt.getLengthOfFormattingRun(0));
-        
-        assertNotNull(rt.getFontOfFormattingRun(1));
-        assertEquals(9, rt.getLengthOfFormattingRun(1));
+            // verify the values to ensure future changes keep the returned information equal
+            XSSFRichTextString rt = (XSSFRichTextString) row.getCell(0).getRichStringCellValue();
+            assertEquals(0, rt.numFormattingRuns());
+            assertNull(rt.getFontOfFormattingRun(0));
+            assertEquals(-1, rt.getLengthOfFormattingRun(0));
 
-        assertNull(rt.getFontOfFormattingRun(2));
-        
-        rt = (XSSFRichTextString) row.getCell(3).getRichStringCellValue();
-        assertEquals(3, rt.numFormattingRuns());
-        assertNull(rt.getFontOfFormattingRun(0));
-        assertEquals(1, rt.getLengthOfFormattingRun(0));
-        
-        assertNotNull(rt.getFontOfFormattingRun(1));
-        assertEquals(3, rt.getLengthOfFormattingRun(1));
-        
-        assertNotNull(rt.getFontOfFormattingRun(2));
-        assertEquals(9, rt.getLengthOfFormattingRun(2));
+            rt = (XSSFRichTextString) row.getCell(1).getRichStringCellValue();
+            assertEquals(0, row.getCell(1).getRichStringCellValue().numFormattingRuns());
+            assertNull(rt.getFontOfFormattingRun(1));
+            assertEquals(-1, rt.getLengthOfFormattingRun(1));
+
+            rt = (XSSFRichTextString) row.getCell(2).getRichStringCellValue();
+            assertEquals(2, rt.numFormattingRuns());
+            assertNotNull(rt.getFontOfFormattingRun(0));
+            assertEquals(4, rt.getLengthOfFormattingRun(0));
+
+            assertNotNull(rt.getFontOfFormattingRun(1));
+            assertEquals(9, rt.getLengthOfFormattingRun(1));
+
+            assertNull(rt.getFontOfFormattingRun(2));
+
+            rt = (XSSFRichTextString) row.getCell(3).getRichStringCellValue();
+            assertEquals(3, rt.numFormattingRuns());
+            assertNull(rt.getFontOfFormattingRun(0));
+            assertEquals(1, rt.getLengthOfFormattingRun(0));
+
+            assertNotNull(rt.getFontOfFormattingRun(1));
+            assertEquals(3, rt.getLengthOfFormattingRun(1));
+
+            assertNotNull(rt.getFontOfFormattingRun(2));
+            assertEquals(9, rt.getLengthOfFormattingRun(2));
+        }
     }
 
     public void testToString() {
         XSSFRichTextString rt = new XSSFRichTextString("Apache POI");
         assertNotNull(rt.toString());
-        
-        // TODO: normally toString() should never return null, should we adjust this?
+
         rt = new XSSFRichTextString();
-        assertNull(rt.toString());
+        assertEquals("", rt.toString());
     }
 
     public void test59008Font() {
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSimpleShape.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSimpleShape.java
index 78c476c5b8..5387b1a7da 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSimpleShape.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSimpleShape.java
@@ -38,8 +38,7 @@ import org.junit.Test;
 public class TestXSSFSimpleShape {
     @Test
     public void testXSSFTextParagraph() throws IOException {
-        XSSFWorkbook wb = new XSSFWorkbook();
-        try {
+        try (XSSFWorkbook wb = new XSSFWorkbook()) {
             XSSFSheet sheet = wb.createSheet();
             XSSFDrawing drawing = sheet.createDrawingPatriarch();
 
@@ -209,8 +208,6 @@ public class TestXSSFSimpleShape {
             // assertEquals(-1, shape.getShapeType());
 
             assertNotNull(shape.getShapeProperties());
-        } finally {
-            wb.close();
         }
     }
 }