From 4c017a0e2782799cb5a53476d890d65e0078c670 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sun, 3 Mar 2019 14:09:40 +0000 Subject: [PATCH] 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 --- .../xssf/usermodel/XSSFRichTextString.java | 15 +- .../poi/xssf/usermodel/XSSFSimpleShape.java | 21 +-- .../poi/xssf/usermodel/TestXSSFCell.java | 11 +- .../usermodel/TestXSSFRichTextString.java | 139 +++++++++--------- .../xssf/usermodel/TestXSSFSimpleShape.java | 5 +- 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 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 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 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\n\n", 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 ? "" : 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 ? "" : 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(); } } } -- 2.39.5