From e623808539126c32d43bd6bcbd67aa12845eab89 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Tue, 31 Oct 2017 09:48:23 +0000 Subject: [PATCH] add test case for bug 61701 and use StringBuilder in more places git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1813863 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/poi/hssf/model/LinkTable.java | 4 +- .../poi/hssf/usermodel/HSSFWorkbook.java | 2 +- .../poi/ss/formula/SheetNameFormatter.java | 67 +++++++++++++++---- .../poi/ss/formula/functions/Address.java | 2 +- .../apache/poi/ss/formula/ptg/Area3DPxg.java | 2 +- .../poi/ss/formula/ptg/Deleted3DPxg.java | 2 +- .../formula/ptg/ExternSheetNameResolver.java | 8 +-- .../apache/poi/ss/formula/ptg/NameXPxg.java | 2 +- .../apache/poi/ss/formula/ptg/Ref3DPxg.java | 2 +- .../org/apache/poi/ss/util/AreaReference.java | 14 ++-- .../org/apache/poi/ss/util/CellReference.java | 4 +- .../org/apache/poi/POIXMLTextExtractor.java | 2 +- .../xssf/extractor/XSSFExcelExtractor.java | 6 +- .../apache/poi/xssf/streaming/SXSSFRow.java | 2 +- .../apache/poi/xssf/usermodel/XSSFName.java | 2 +- .../poi/xssf/usermodel/XSSFWorkbook.java | 6 +- .../org/apache/poi/xssf/AllXSSFTests.java | 4 +- .../xssf/eventusermodel/TestXSSFReader.java | 35 ++++++++-- .../org/apache/poi/POIDataSamples.java | 2 +- 19 files changed, 119 insertions(+), 49 deletions(-) diff --git a/src/java/org/apache/poi/hssf/model/LinkTable.java b/src/java/org/apache/poi/hssf/model/LinkTable.java index 0dc7f53eb9..b55e7b952f 100644 --- a/src/java/org/apache/poi/hssf/model/LinkTable.java +++ b/src/java/org/apache/poi/hssf/model/LinkTable.java @@ -547,8 +547,8 @@ final class LinkTable { // Workbook scoped name, not actually external after all NameRecord nr = getNameRecord(definedNameIndex); int sheetNumber = nr.getSheetNumber(); - - StringBuffer text = new StringBuffer(); + + StringBuilder text = new StringBuilder(64); if (sheetNumber > 0) { String sheetName = workbook.getSheetName(sheetNumber-1); SheetNameFormatter.appendFormat(text, sheetName); diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java index 01324e5716..8b0697483f 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java @@ -1650,7 +1650,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss // adding one here because 0 indicates a global named region; doesn't make sense for print areas } String[] parts = COMMA_PATTERN.split(reference); - StringBuffer sb = new StringBuffer(32); + StringBuilder sb = new StringBuilder(32); for (int i = 0; i < parts.length; i++) { if(i>0) { sb.append(","); diff --git a/src/java/org/apache/poi/ss/formula/SheetNameFormatter.java b/src/java/org/apache/poi/ss/formula/SheetNameFormatter.java index 7f4f5b86cb..c6e13c3110 100644 --- a/src/java/org/apache/poi/ss/formula/SheetNameFormatter.java +++ b/src/java/org/apache/poi/ss/formula/SheetNameFormatter.java @@ -47,15 +47,17 @@ public final class SheetNameFormatter { * sheet name will be converted to double single quotes (''). */ public static String format(String rawSheetName) { - StringBuffer sb = new StringBuffer(rawSheetName.length() + 2); + StringBuilder sb = new StringBuilder(rawSheetName.length() + 2); appendFormat(sb, rawSheetName); return sb.toString(); } /** * Convenience method for ({@link #format(String)}) when a StringBuffer is already available. - * - * @param out - sheet name will be appended here possibly with delimiting quotes + * + * @param out - sheet name will be appended here possibly with delimiting quotes + * @param rawSheetName - sheet name + * @deprecated use appendFormat(StringBuilder out, String rawSheetName) instead */ public static void appendFormat(StringBuffer out, String rawSheetName) { boolean needsQuotes = needsDelimiting(rawSheetName); @@ -67,6 +69,10 @@ public final class SheetNameFormatter { out.append(rawSheetName); } } + + /** + * @deprecated use appendFormat(StringBuilder out, String workbookName, String rawSheetName) instead + */ public static void appendFormat(StringBuffer out, String workbookName, String rawSheetName) { boolean needsQuotes = needsDelimiting(workbookName) || needsDelimiting(rawSheetName); if(needsQuotes) { @@ -84,17 +90,54 @@ public final class SheetNameFormatter { } } - private static void appendAndEscape(StringBuffer sb, String rawSheetName) { - int len = rawSheetName.length(); - for(int i=0; i= 0) { sb.append('['); sb.append(externalWorkbookNumber); diff --git a/src/java/org/apache/poi/ss/formula/ptg/Deleted3DPxg.java b/src/java/org/apache/poi/ss/formula/ptg/Deleted3DPxg.java index b1b58d6eab..84e4a0de16 100644 --- a/src/java/org/apache/poi/ss/formula/ptg/Deleted3DPxg.java +++ b/src/java/org/apache/poi/ss/formula/ptg/Deleted3DPxg.java @@ -65,7 +65,7 @@ public final class Deleted3DPxg extends OperandPtg implements Pxg { } public String toFormulaString() { - StringBuffer sb = new StringBuffer(); + StringBuilder sb = new StringBuilder(64); if (externalWorkbookNumber >= 0) { sb.append('['); sb.append(externalWorkbookNumber); diff --git a/src/java/org/apache/poi/ss/formula/ptg/ExternSheetNameResolver.java b/src/java/org/apache/poi/ss/formula/ptg/ExternSheetNameResolver.java index a61d1d5752..c1989e17db 100644 --- a/src/java/org/apache/poi/ss/formula/ptg/ExternSheetNameResolver.java +++ b/src/java/org/apache/poi/ss/formula/ptg/ExternSheetNameResolver.java @@ -32,15 +32,15 @@ final class ExternSheetNameResolver { public static String prependSheetName(FormulaRenderingWorkbook book, int field_1_index_extern_sheet, String cellRefText) { ExternalSheet externalSheet = book.getExternalSheet(field_1_index_extern_sheet); - StringBuffer sb; + StringBuilder sb; if (externalSheet != null) { String wbName = externalSheet.getWorkbookName(); String sheetName = externalSheet.getSheetName(); if (wbName != null) { - sb = new StringBuffer(wbName.length() + sheetName.length() + cellRefText.length() + 4); + sb = new StringBuilder(wbName.length() + sheetName.length() + cellRefText.length() + 4); SheetNameFormatter.appendFormat(sb, wbName, sheetName); } else { - sb = new StringBuffer(sheetName.length() + cellRefText.length() + 4); + sb = new StringBuilder(sheetName.length() + cellRefText.length() + 4); SheetNameFormatter.appendFormat(sb, sheetName); } if (externalSheet instanceof ExternalSheetRange) { @@ -53,7 +53,7 @@ final class ExternSheetNameResolver { } else { String firstSheetName = book.getSheetFirstNameByExternSheet(field_1_index_extern_sheet); String lastSheetName = book.getSheetLastNameByExternSheet(field_1_index_extern_sheet); - sb = new StringBuffer(firstSheetName.length() + cellRefText.length() + 4); + sb = new StringBuilder(firstSheetName.length() + cellRefText.length() + 4); if (firstSheetName.length() < 1) { // What excel does if sheet has been deleted sb.append("#REF"); // note - '!' added just once below diff --git a/src/java/org/apache/poi/ss/formula/ptg/NameXPxg.java b/src/java/org/apache/poi/ss/formula/ptg/NameXPxg.java index 8969ad47bf..9da79beec8 100644 --- a/src/java/org/apache/poi/ss/formula/ptg/NameXPxg.java +++ b/src/java/org/apache/poi/ss/formula/ptg/NameXPxg.java @@ -76,7 +76,7 @@ public final class NameXPxg extends OperandPtg implements Pxg { } public String toFormulaString() { - StringBuffer sb = new StringBuffer(); + StringBuilder sb = new StringBuilder(64); boolean needsExclamation = false; if (externalWorkbookNumber >= 0) { sb.append('['); diff --git a/src/java/org/apache/poi/ss/formula/ptg/Ref3DPxg.java b/src/java/org/apache/poi/ss/formula/ptg/Ref3DPxg.java index 3b78c5a4b0..67f73b360d 100644 --- a/src/java/org/apache/poi/ss/formula/ptg/Ref3DPxg.java +++ b/src/java/org/apache/poi/ss/formula/ptg/Ref3DPxg.java @@ -100,7 +100,7 @@ public final class Ref3DPxg extends RefPtgBase implements Pxg3D { } public String toFormulaString() { - StringBuffer sb = new StringBuffer(); + StringBuilder sb = new StringBuilder(64); if (externalWorkbookNumber >= 0) { sb.append('['); sb.append(externalWorkbookNumber); diff --git a/src/java/org/apache/poi/ss/util/AreaReference.java b/src/java/org/apache/poi/ss/util/AreaReference.java index 7cf3b73c41..821413af11 100644 --- a/src/java/org/apache/poi/ss/util/AreaReference.java +++ b/src/java/org/apache/poi/ss/util/AreaReference.java @@ -295,8 +295,8 @@ public class AreaReference { + ":" + CellReference.convertNumToColString(_lastCell.getCol()); } - - StringBuffer sb = new StringBuffer(32); + + StringBuilder sb = new StringBuilder(32); sb.append(_firstCell.formatAsString()); if(!_isSingleCell) { sb.append(CELL_DELIMITER); @@ -311,10 +311,14 @@ public class AreaReference { } public String toString() { - StringBuffer sb = new StringBuffer(64); + StringBuilder sb = new StringBuilder(64); sb.append(getClass().getName()).append(" ["); - sb.append(formatAsString()); - sb.append("]"); + try { + sb.append(formatAsString()); + } catch(Exception e) { + sb.append(e.toString()); + } + sb.append(']'); return sb.toString(); } diff --git a/src/java/org/apache/poi/ss/util/CellReference.java b/src/java/org/apache/poi/ss/util/CellReference.java index b6be13936d..d14670221f 100644 --- a/src/java/org/apache/poi/ss/util/CellReference.java +++ b/src/java/org/apache/poi/ss/util/CellReference.java @@ -484,7 +484,7 @@ public class CellReference { * @return the text representation of this cell reference as it would appear in a formula. */ public String formatAsString() { - StringBuffer sb = new StringBuffer(32); + StringBuilder sb = new StringBuilder(32); if(_sheetName != null) { SheetNameFormatter.appendFormat(sb, _sheetName); sb.append(SHEET_NAME_DELIMITER); @@ -523,7 +523,7 @@ public class CellReference { * Appends cell reference with '$' markers for absolute values as required. * Sheet name is not included. */ - /* package */ void appendCellReference(StringBuffer sb) { + /* package */ void appendCellReference(StringBuilder sb) { if (_colIndex != -1) { if(_isColAbs) { sb.append(ABSOLUTE_REFERENCE_MARKER); diff --git a/src/ooxml/java/org/apache/poi/POIXMLTextExtractor.java b/src/ooxml/java/org/apache/poi/POIXMLTextExtractor.java index 954feb80ef..26b2cd84c2 100644 --- a/src/ooxml/java/org/apache/poi/POIXMLTextExtractor.java +++ b/src/ooxml/java/org/apache/poi/POIXMLTextExtractor.java @@ -104,7 +104,7 @@ public abstract class POIXMLTextExtractor extends POITextExtractor { super.close(); } - protected void checkMaxTextSize(StringBuffer text, String string) { + protected void checkMaxTextSize(CharSequence text, String string) { if(string == null) { return; } diff --git a/src/ooxml/java/org/apache/poi/xssf/extractor/XSSFExcelExtractor.java b/src/ooxml/java/org/apache/poi/xssf/extractor/XSSFExcelExtractor.java index bb095c78d5..81b93ab463 100644 --- a/src/ooxml/java/org/apache/poi/xssf/extractor/XSSFExcelExtractor.java +++ b/src/ooxml/java/org/apache/poi/xssf/extractor/XSSFExcelExtractor.java @@ -135,7 +135,7 @@ public class XSSFExcelExtractor extends POIXMLTextExtractor formatter = new DataFormatter(locale); } - StringBuffer text = new StringBuffer(); + StringBuilder text = new StringBuilder(64); for(Sheet sh : workbook) { XSSFSheet sheet = (XSSFSheet) sh; if(includeSheetNames) { @@ -229,13 +229,13 @@ public class XSSFExcelExtractor extends POIXMLTextExtractor return text.toString(); } - private void handleStringCell(StringBuffer text, Cell cell) { + private void handleStringCell(StringBuilder text, Cell cell) { String contents = cell.getRichStringCellValue().getString(); checkMaxTextSize(text, contents); text.append(contents); } - private void handleNonStringCell(StringBuffer text, Cell cell, DataFormatter formatter) { + private void handleNonStringCell(StringBuilder text, Cell cell, DataFormatter formatter) { CellType type = cell.getCellType(); if (type == CellType.FORMULA) { type = cell.getCachedFormulaResultType(); diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFRow.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFRow.java index 711cd48945..c8d2f7674f 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFRow.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFRow.java @@ -121,7 +121,7 @@ public class SXSSFRow implements Row, Comparable { return createCell(column, CellType.BLANK); } - + /** * Use this to create new cells within the row and return it. *

diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java index 575b01c994..e56af69d9a 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFName.java @@ -283,7 +283,7 @@ public final class XSSFName implements Name { * Get the sheets name which this named range is referenced to * * @return sheet name, which this named range referred to. - * Empty string if the referenced sheet name weas not found. + * Empty string if the referenced sheet name was not found. */ public String getSheetName() { if (_ctName.isSetLocalSheetId()) { diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java index 67a82510de..1916ef50b7 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java @@ -1490,13 +1490,13 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { //short externSheetIndex = getWorkbook().checkExternSheet(sheetIndex); //name.setExternSheetNumber(externSheetIndex); String[] parts = COMMA_PATTERN.split(reference); - StringBuffer sb = new StringBuffer(32); + StringBuilder sb = new StringBuilder(32); for (int i = 0; i < parts.length; i++) { if(i>0) { - sb.append(","); + sb.append(','); } SheetNameFormatter.appendFormat(sb, getSheetName(sheetIndex)); - sb.append("!"); + sb.append('!'); sb.append(parts[i]); } name.setRefersToFormula(sb.toString()); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/AllXSSFTests.java b/src/ooxml/testcases/org/apache/poi/xssf/AllXSSFTests.java index c6e2f7ecaa..93f35aac49 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/AllXSSFTests.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/AllXSSFTests.java @@ -18,7 +18,6 @@ package org.apache.poi.xssf; import org.apache.poi.ss.format.TestCellFormatPart; -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; @@ -29,14 +28,13 @@ import org.apache.poi.xssf.util.TestNumericRanges; import org.junit.runner.RunWith; import org.junit.runners.Suite; - /** * Collects all tests for org.apache.poi.xssf and sub-packages. */ @RunWith(Suite.class) @Suite.SuiteClasses({ AllXSSFUsermodelTests.class, - TestXSSFReader.class, + //TestXSSFReader.class, //converted to junit4 TestXSSFExcelExtractor.class, TestLoadSaveXSSF.class, TestCommentsTable.class, diff --git a/src/ooxml/testcases/org/apache/poi/xssf/eventusermodel/TestXSSFReader.java b/src/ooxml/testcases/org/apache/poi/xssf/eventusermodel/TestXSSFReader.java index c70a24cebc..5e27ad61c0 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/eventusermodel/TestXSSFReader.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/eventusermodel/TestXSSFReader.java @@ -19,6 +19,7 @@ package org.apache.poi.xssf.eventusermodel; import static org.apache.poi.POITestCase.assertContains; import static org.apache.poi.POITestCase.assertNotContained; +import static org.junit.Assert.*; import java.io.InputStream; import java.util.Iterator; @@ -29,6 +30,8 @@ import java.util.HashSet; import org.apache.poi.POIDataSamples; import org.apache.poi.POIXMLException; import org.apache.poi.openxml4j.opc.OPCPackage; +import org.apache.poi.ss.usermodel.Name; +import org.apache.poi.ss.usermodel.Workbook; import org.apache.poi.util.IOUtils; import org.apache.poi.xssf.XSSFTestDataSamples; import org.apache.poi.xssf.model.CommentsTable; @@ -36,21 +39,23 @@ import org.apache.poi.xssf.model.StylesTable; import org.apache.poi.xssf.usermodel.XSSFRichTextString; import org.apache.poi.xssf.usermodel.XSSFShape; import org.apache.poi.xssf.usermodel.XSSFSimpleShape; - -import junit.framework.TestCase; +import org.junit.Ignore; +import org.junit.Test; /** * Tests for {@link XSSFReader} */ -public final class TestXSSFReader extends TestCase { +public final class TestXSSFReader { + private static POIDataSamples _ssTests = POIDataSamples.getSpreadSheetInstance(); + @Test public void testGetBits() throws Exception { OPCPackage pkg = OPCPackage.open(_ssTests.openResourceAsStream("SampleSS.xlsx")); XSSFReader r = new XSSFReader(pkg); - assertNotNull(r.getWorkbookData()); + assertNotNull(r.getWorkbookData()); assertNotNull(r.getSharedStringsData()); assertNotNull(r.getStylesData()); @@ -58,6 +63,7 @@ public final class TestXSSFReader extends TestCase { assertNotNull(r.getStylesTable()); } + @Test public void testStyles() throws Exception { OPCPackage pkg = OPCPackage.open(_ssTests.openResourceAsStream("SampleSS.xlsx")); @@ -74,6 +80,7 @@ public final class TestXSSFReader extends TestCase { assertNotNull(r.getThemesData()); } + @Test public void testStrings() throws Exception { OPCPackage pkg = OPCPackage.open(_ssTests.openResourceAsStream("SampleSS.xlsx")); @@ -83,6 +90,7 @@ public final class TestXSSFReader extends TestCase { assertEquals("Test spreadsheet", new XSSFRichTextString(r.getSharedStringsTable().getEntryAt(0)).toString()); } + @Test public void testSheets() throws Exception { OPCPackage pkg = OPCPackage.open(_ssTests.openResourceAsStream("SampleSS.xlsx")); @@ -115,6 +123,7 @@ public final class TestXSSFReader extends TestCase { * Check that the sheet iterator returns sheets in the logical order * (as they are defined in the workbook.xml) */ + @Test public void testOrderOfSheets() throws Exception { OPCPackage pkg = OPCPackage.open(_ssTests.openResourceAsStream("reordered_sheets.xlsx")); @@ -134,7 +143,8 @@ public final class TestXSSFReader extends TestCase { } assertEquals(4, count); } - + + @Test public void testComments() throws Exception { OPCPackage pkg = XSSFTestDataSamples.openSamplePackage("comments.xlsx"); XSSFReader r = new XSSFReader(pkg); @@ -163,6 +173,7 @@ public final class TestXSSFReader extends TestCase { * XSSFReader method * @throws Exception */ + @Test public void test50119() throws Exception { OPCPackage pkg = XSSFTestDataSamples.openSamplePackage("WithChartSheet.xlsx"); XSSFReader r = new XSSFReader(pkg); @@ -180,6 +191,7 @@ public final class TestXSSFReader extends TestCase { * * @throws Exception */ + @Test public void testShapes() throws Exception { OPCPackage pkg = XSSFTestDataSamples.openSamplePackage("WithTextBox.xlsx"); XSSFReader r = new XSSFReader(pkg); @@ -208,6 +220,7 @@ public final class TestXSSFReader extends TestCase { return sb.toString(); } + @Test public void testBug57914() throws Exception { OPCPackage pkg = XSSFTestDataSamples.openSamplePackage("57914.xlsx"); final XSSFReader r; @@ -234,6 +247,7 @@ public final class TestXSSFReader extends TestCase { * NPE from XSSFReader$SheetIterator. on XLSX files generated by * the openpyxl library */ + @Test public void test58747() throws Exception { OPCPackage pkg = XSSFTestDataSamples.openSamplePackage("58747.xlsx"); ReadOnlySharedStringsTable strings = new ReadOnlySharedStringsTable(pkg); @@ -256,6 +270,7 @@ public final class TestXSSFReader extends TestCase { * NPE when sheet has no relationship id in the workbook * 60825 */ + @Test public void testSheetWithNoRelationshipId() throws Exception { OPCPackage pkg = XSSFTestDataSamples.openSamplePackage("60825.xlsx"); ReadOnlySharedStringsTable strings = new ReadOnlySharedStringsTable(pkg); @@ -283,6 +298,7 @@ public final class TestXSSFReader extends TestCase { * While this one works correctly: * <sheet name="Sheet6" sheetId="4" r:id="rId6"/> */ + @Test public void test61034() throws Exception { OPCPackage pkg = XSSFTestDataSamples.openSamplePackage("61034.xlsx"); XSSFReader reader = new XSSFReader(pkg); @@ -297,4 +313,13 @@ public final class TestXSSFReader extends TestCase { } pkg.close(); } + + @Test + @Ignore("until we fix issue https://bz.apache.org/bugzilla/show_bug.cgi?id=61701") + public void test61701() throws Exception { + try(Workbook workbook = XSSFTestDataSamples.openSampleWorkbook("simple-table-named-range.xlsx")) { + Name name = workbook.getName("total"); + System.out.println("workbook.getName(\"total\").getSheetName() returned: " + name.getSheetName()); + } + } } diff --git a/src/testcases/org/apache/poi/POIDataSamples.java b/src/testcases/org/apache/poi/POIDataSamples.java index 4b764c2ee3..c4a093fe41 100644 --- a/src/testcases/org/apache/poi/POIDataSamples.java +++ b/src/testcases/org/apache/poi/POIDataSamples.java @@ -206,7 +206,7 @@ public final class POIDataSamples { } File dataDir = new File(dataDirName, _moduleDir); if (!dataDir.exists()) { - throw new RuntimeException("Data dir '" + _moduleDir + " does not exist"); + throw new RuntimeException("Data dir '" + _moduleDir + "' does not exist"); } // convert to canonical file, to make any subsequent error messages // clearer. -- 2.39.5