diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 202938cffd..71143cde3d 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 49928 - allow overridden built-in formats in HSSFCellStyle 50607 - Added implementation for CLEAN(), CHAR() and ADDRESS() 50587 - Improved documentation on user-defined functions Inside ExtractorFactory, support finding embedded OOXML documents and providing extractors for them diff --git a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java index 12bd39d633..62796a62f2 100644 --- a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java +++ b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java @@ -88,6 +88,7 @@ import org.apache.poi.ss.formula.ptg.Ptg; import org.apache.poi.hssf.util.HSSFColor; import org.apache.poi.ss.formula.EvaluationWorkbook.ExternalName; import org.apache.poi.ss.formula.EvaluationWorkbook.ExternalSheet; +import org.apache.poi.ss.usermodel.BuiltinFormats; import org.apache.poi.util.Internal; import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; @@ -1284,15 +1285,16 @@ public final class InternalWorkbook { // we'll need multiple editions for // the different formats + switch (id) { - case 0: return new FormatRecord(5, "\"$\"#,##0_);\\(\"$\"#,##0\\)"); - case 1: return new FormatRecord(6, "\"$\"#,##0_);[Red]\\(\"$\"#,##0\\)"); - case 2: return new FormatRecord(7, "\"$\"#,##0.00_);\\(\"$\"#,##0.00\\)"); - case 3: return new FormatRecord(8, "\"$\"#,##0.00_);[Red]\\(\"$\"#,##0.00\\)"); - case 4: return new FormatRecord(0x2a, "_(\"$\"* #,##0_);_(\"$\"* \\(#,##0\\);_(\"$\"* \"-\"_);_(@_)"); - case 5: return new FormatRecord(0x29, "_(* #,##0_);_(* \\(#,##0\\);_(* \"-\"_);_(@_)"); - case 6: return new FormatRecord(0x2c, "_(\"$\"* #,##0.00_);_(\"$\"* \\(#,##0.00\\);_(\"$\"* \"-\"??_);_(@_)"); - case 7: return new FormatRecord(0x2b, "_(* #,##0.00_);_(* \\(#,##0.00\\);_(* \"-\"??_);_(@_)"); + case 0: return new FormatRecord(5, BuiltinFormats.getBuiltinFormat(5)); + case 1: return new FormatRecord(6, BuiltinFormats.getBuiltinFormat(6)); + case 2: return new FormatRecord(7, BuiltinFormats.getBuiltinFormat(7)); + case 3: return new FormatRecord(8, BuiltinFormats.getBuiltinFormat(8)); + case 4: return new FormatRecord(0x2a, BuiltinFormats.getBuiltinFormat(0x2a)); + case 5: return new FormatRecord(0x29, BuiltinFormats.getBuiltinFormat(0x29)); + case 6: return new FormatRecord(0x2c, BuiltinFormats.getBuiltinFormat(0x2c)); + case 7: return new FormatRecord(0x2b, BuiltinFormats.getBuiltinFormat(0x2b)); } throw new IllegalArgumentException("Unexpected id " + id); } diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFDataFormat.java b/src/java/org/apache/poi/hssf/usermodel/HSSFDataFormat.java index beb22a2a41..382452ca97 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFDataFormat.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFDataFormat.java @@ -71,9 +71,7 @@ public final class HSSFDataFormat implements DataFormat { Iterator i = workbook.getFormats().iterator(); while (i.hasNext()) { FormatRecord r = i.next(); - if (_formats.size() < r.getIndexCode() + 1) { - _formats.setSize(r.getIndexCode() + 1); - } + ensureFormatsSize(r.getIndexCode()); _formats.set(r.getIndexCode(), r.getFormatString()); } } @@ -100,7 +98,7 @@ public final class HSSFDataFormat implements DataFormat { * @return index of format. */ public short getFormat(String pFormat) { - + // Normalise the format string String format; if (pFormat.toUpperCase().equals("TEXT")) { format = "@"; @@ -108,31 +106,31 @@ public final class HSSFDataFormat implements DataFormat { format = pFormat; } + // Merge in the built in formats if we haven't already if (!_movedBuiltins) { for (int i=0; i<_builtinFormats.length; i++) { - if (_formats.size() < i + 1) { - _formats.setSize(i + 1); + ensureFormatsSize(i); + if (_formats.get(i) == null) { + _formats.set(i, _builtinFormats[i]); + } else { + // The workbook overrides this default format } - _formats.set(i, _builtinFormats[i]); } _movedBuiltins = true; } - ListIterator i; - i = _formats.listIterator(); - int ind; - while (i.hasNext()) { - ind = i.nextIndex(); - if (format.equals(i.next())) { - return (short) ind; - } + + // See if we can find it + for(int i=0; i<_formats.size(); i++) { + if(format.equals(_formats.get(i))) { + return (short)i; + } } - ind = _workbook.getFormat(format, true); - if (_formats.size() <= ind) - _formats.setSize(ind + 1); - _formats.set(ind, format); - - return (short) ind; + // We can't find it, so add it as a new one + short index = _workbook.getFormat(format, true); + ensureFormatsSize(index); + _formats.set(index, format); + return index; } /** @@ -144,10 +142,19 @@ public final class HSSFDataFormat implements DataFormat { if (_movedBuiltins) { return _formats.get(index); } + + String fmt = _formats.get(index); if (_builtinFormats.length > index && _builtinFormats[index] != null) { - return _builtinFormats[index]; + // It's in the built in range + if (fmt != null) { + // It's been overriden, use that value + return fmt; + } else { + // Standard built in format + return _builtinFormats[index]; + } } - return _formats.get(index); + return fmt; } /** @@ -166,4 +173,14 @@ public final class HSSFDataFormat implements DataFormat { public static int getNumberOfBuiltinBuiltinFormats() { return _builtinFormats.length; } + + /** + * Ensures that the formats list can hold entries + * up to and including the entry with this index + */ + private void ensureFormatsSize(int index) { + if(_formats.size() <= index) { + _formats.setSize(index+1); + } + } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataFormat.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataFormat.java index 3769ccf196..32bcaef785 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataFormat.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataFormat.java @@ -19,7 +19,7 @@ package org.apache.poi.xssf.usermodel; import org.apache.poi.ss.usermodel.BaseTestDataFormat; import org.apache.poi.ss.usermodel.BuiltinFormats; -import org.apache.poi.ss.usermodel.DataFormatter; +import org.apache.poi.ss.usermodel.DataFormat; import org.apache.poi.xssf.XSSFITestDataProvider; import org.apache.poi.xssf.XSSFTestDataSamples; @@ -37,33 +37,16 @@ public final class TestXSSFDataFormat extends BaseTestDataFormat { */ public void test49928(){ XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("49928.xlsx"); - DataFormatter df = new DataFormatter(); - - XSSFSheet sheet = wb.getSheetAt(0); - XSSFCell cell = sheet.getRow(0).getCell(0); - XSSFCellStyle style = cell.getCellStyle(); - - String poundFmt = "\"\u00a3\"#,##0;[Red]\\-\"\u00a3\"#,##0"; - // not expected normally, id of a custom format should be gerater - // than BuiltinFormats.FIRST_USER_DEFINED_FORMAT_INDEX - short poundFmtIdx = 6; - - assertEquals(poundFmt, style.getDataFormatString()); - assertEquals(poundFmtIdx, style.getDataFormat()); - assertEquals("\u00a31", df.formatCellValue(cell)); - - - XSSFDataFormat dataFormat = wb.createDataFormat(); - assertEquals(poundFmtIdx, dataFormat.getFormat(poundFmt)); - assertEquals(poundFmt, dataFormat.getFormat(poundFmtIdx)); + doTest49928Core(wb); // an attempt to register an existing format returns its index + int poundFmtIdx = wb.getSheetAt(0).getRow(0).getCell(0).getCellStyle().getDataFormat(); assertEquals(poundFmtIdx, wb.getStylesSource().putNumberFormat(poundFmt)); // now create a custom format with Pound (\u00a3) + DataFormat dataFormat = wb.createDataFormat(); short customFmtIdx = dataFormat.getFormat("\u00a3##.00[Yellow]"); assertTrue(customFmtIdx > BuiltinFormats.FIRST_USER_DEFINED_FORMAT_INDEX ); assertEquals("\u00a3##.00[Yellow]", dataFormat.getFormat(customFmtIdx)); - } } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormat.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormat.java index c04f162205..6eb9b8dbbb 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormat.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormat.java @@ -18,7 +18,10 @@ package org.apache.poi.hssf.usermodel; import org.apache.poi.hssf.HSSFITestDataProvider; +import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.ss.usermodel.BaseTestDataFormat; +import org.apache.poi.ss.usermodel.BuiltinFormats; +import org.apache.poi.ss.usermodel.DataFormat; /** * Tests for {@link HSSFDataFormat} @@ -28,4 +31,22 @@ public final class TestHSSFDataFormat extends BaseTestDataFormat { public TestHSSFDataFormat() { super(HSSFITestDataProvider.instance); } + + /** + * [Bug 49928] formatCellValue returns incorrect value for \u00a3 formatted cells + */ + public void test49928(){ + HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("49928.xls"); + doTest49928Core(wb); + + // an attempt to register an existing format returns its index + int poundFmtIdx = wb.getSheetAt(0).getRow(0).getCell(0).getCellStyle().getDataFormat(); + assertEquals(poundFmtIdx, wb.createDataFormat().getFormat(poundFmt)); + + // now create a custom format with Pound (\u00a3) + DataFormat dataFormat = wb.createDataFormat(); + short customFmtIdx = dataFormat.getFormat("\u00a3##.00[Yellow]"); + assertTrue(customFmtIdx >= BuiltinFormats.FIRST_USER_DEFINED_FORMAT_INDEX ); + assertEquals("\u00a3##.00[Yellow]", dataFormat.getFormat(customFmtIdx)); + } } diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestDataFormat.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestDataFormat.java index 46f8204f11..cd161a52dc 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestDataFormat.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestDataFormat.java @@ -20,6 +20,12 @@ package org.apache.poi.ss.usermodel; import junit.framework.TestCase; import org.apache.poi.ss.ITestDataProvider; +import org.apache.poi.xssf.XSSFTestDataSamples; +import org.apache.poi.xssf.usermodel.XSSFCell; +import org.apache.poi.xssf.usermodel.XSSFCellStyle; +import org.apache.poi.xssf.usermodel.XSSFDataFormat; +import org.apache.poi.xssf.usermodel.XSSFSheet; +import org.apache.poi.xssf.usermodel.XSSFWorkbook; /** * Tests of implementation of {@link DataFormat} @@ -60,4 +66,31 @@ public abstract class BaseTestDataFormat extends TestCase { //read and verify the string representation assertEquals(customFmt, df.getFormat((short)customIdx)); } + + /** + * [Bug 49928] formatCellValue returns incorrect value for \u00a3 formatted cells + */ + public abstract void test49928(); + protected final String poundFmt = "\"\u00a3\"#,##0;[Red]\\-\"\u00a3\"#,##0"; + public void doTest49928Core(Workbook wb){ + DataFormatter df = new DataFormatter(); + + Sheet sheet = wb.getSheetAt(0); + Cell cell = sheet.getRow(0).getCell(0); + CellStyle style = cell.getCellStyle(); + + String poundFmt = "\"\u00a3\"#,##0;[Red]\\-\"\u00a3\"#,##0"; + // not expected normally, id of a custom format should be greater + // than BuiltinFormats.FIRST_USER_DEFINED_FORMAT_INDEX + short poundFmtIdx = 6; + + assertEquals(poundFmt, style.getDataFormatString()); + assertEquals(poundFmtIdx, style.getDataFormat()); + assertEquals("\u00a31", df.formatCellValue(cell)); + + + DataFormat dataFormat = wb.createDataFormat(); + assertEquals(poundFmtIdx, dataFormat.getFormat(poundFmt)); + assertEquals(poundFmt, dataFormat.getFormat(poundFmtIdx)); + } } diff --git a/test-data/spreadsheet/49928.xls b/test-data/spreadsheet/49928.xls new file mode 100644 index 0000000000..d7249e7bda Binary files /dev/null and b/test-data/spreadsheet/49928.xls differ