diff --git a/src/java/org/apache/poi/ss/usermodel/DataFormatter.java b/src/java/org/apache/poi/ss/usermodel/DataFormatter.java index 2f414a1d9..add3be8ce 100644 --- a/src/java/org/apache/poi/ss/usermodel/DataFormatter.java +++ b/src/java/org/apache/poi/ss/usermodel/DataFormatter.java @@ -40,8 +40,12 @@ import java.util.Observer; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.poi.ss.format.CellFormat; +import org.apache.poi.ss.format.CellFormatResult; import org.apache.poi.ss.util.NumberToTextConverter; import org.apache.poi.util.LocaleUtil; +import org.apache.poi.util.POILogFactory; +import org.apache.poi.util.POILogger; /** @@ -197,6 +201,9 @@ public class DataFormatter implements Observer { /** the Observable to notify, when the locale has been changed */ private final LocaleChangeObservable localeChangedObervable = new LocaleChangeObservable(); + /** For logging any problems we find */ + private static POILogger logger = POILogFactory.getLogger(DataFormatter.class); + /** * Creates a formatter using the {@link Locale#getDefault() default locale}. */ @@ -270,28 +277,30 @@ public class DataFormatter implements Observer { // String formatStr = (i < formatBits.length) ? formatBits[i] : formatBits[0]; String formatStr = formatStrIn; - // Excel supports positive/negative/zero, but java - // doesn't, so we need to do it specially - final int firstAt = formatStr.indexOf(';'); - final int lastAt = formatStr.lastIndexOf(';'); - // p and p;n are ok by default. p;n;z and p;n;z;s need to be fixed. - if (firstAt != -1 && firstAt != lastAt) { - final int secondAt = formatStr.indexOf(';', firstAt + 1); - if (secondAt == lastAt) { // p;n;z - if (cellValue == 0.0) { - formatStr = formatStr.substring(lastAt + 1); - } else { - formatStr = formatStr.substring(0, lastAt); - } - } else { - if (cellValue == 0.0) { // p;n;z;s - formatStr = formatStr.substring(secondAt + 1, lastAt); - } else { - formatStr = formatStr.substring(0, secondAt); + + // Excel supports 3+ part conditional data formats, eg positive/negative/zero, + // or (>1000),(>0),(0),(negative). As Java doesn't handle these kinds + // of different formats for different ranges, just +ve/-ve, we need to + // handle these ourselves in a special way. + // For now, if we detect 3+ parts, we call out to CellFormat to handle it + // TODO Going forward, we should really merge the logic between the two classes + if (formatStr.indexOf(";") != -1 && + formatStr.indexOf(';') != formatStr.lastIndexOf(';')) { + try { + // Ask CellFormat to get a formatter for it + CellFormat cfmt = CellFormat.getInstance(formatStr); + // CellFormat requires callers to identify date vs not, so do so + Object cellValueO = Double.valueOf(cellValue); + if (DateUtil.isADateFormat(formatIndex, formatStr)) { + cellValueO = DateUtil.getJavaDate(cellValue); } + // Wrap and return (non-cachable - CellFormat does that) + return new CellFormatResultWrapper( cfmt.apply(cellValueO) ); + } catch (Exception e) { + logger.log(POILogger.WARN, "Formatting failed as " + formatStr + ", falling back", e); } } - + // Excel's # with value 0 will output empty where Java will output 0. This hack removes the # from the format. if (emulateCsv && cellValue == 0.0 && formatStr.contains("#") && !formatStr.contains("0")) { formatStr = formatStr.replaceAll("#", ""); @@ -310,7 +319,6 @@ public class DataFormatter implements Observer { // Build a formatter, and cache it format = createFormat(cellValue, formatIndex, formatStr); - formats.put(formatStr, format); return format; } @@ -1116,4 +1124,21 @@ public class DataFormatter implements Observer { return df.parseObject(source, pos); } } + /** + * Workaround until we merge {@link DataFormatter} with {@link CellFormat}. + * Constant, non-cachable wrapper around a {@link CellFormatResult} + */ + @SuppressWarnings("serial") + private static final class CellFormatResultWrapper extends Format { + private final CellFormatResult result; + private CellFormatResultWrapper(CellFormatResult result) { + this.result = result; + } + public StringBuffer format(Object obj, StringBuffer toAppendTo, FieldPosition pos) { + return toAppendTo.append(result.text); + } + public Object parseObject(String source, ParsePosition pos) { + return null; // Not supported + } + } } 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 32bcaef78..d7d77d829 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataFormat.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataFormat.java @@ -35,7 +35,7 @@ public final class TestXSSFDataFormat extends BaseTestDataFormat { /** * [Bug 49928] formatCellValue returns incorrect value for \u00a3 formatted cells */ - public void test49928(){ + public void test49928() { XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("49928.xlsx"); doTest49928Core(wb); @@ -49,4 +49,12 @@ public final class TestXSSFDataFormat extends BaseTestDataFormat { assertTrue(customFmtIdx > BuiltinFormats.FIRST_USER_DEFINED_FORMAT_INDEX ); assertEquals("\u00a3##.00[Yellow]", dataFormat.getFormat(customFmtIdx)); } + + /** + * [Bug 58532] Handle formats that go numnum, numK, numM etc + */ + public void test58532() { + XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("FormatKM.xlsx"); + doTest58532Core(wb); + } } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormat.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormat.java index 8761ecf10..6b6fc81a5 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormat.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormat.java @@ -51,6 +51,14 @@ public final class TestHSSFDataFormat extends BaseTestDataFormat { assertEquals("\u00a3##.00[Yellow]", dataFormat.getFormat(customFmtIdx)); } + /** + * [Bug 58532] Handle formats that go numnum, numK, numM etc + */ + public void test58532() { + HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("FormatKM.xls"); + doTest58532Core(wb); + } + /** * Bug 51378: getDataFormatString method call crashes when reading the test file */ diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormatter.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormatter.java index 85ceee2f9..f42074e99 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormatter.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFDataFormatter.java @@ -265,8 +265,8 @@ public final class TestHSSFDataFormatter { assertTrue( ! "555.47431".equals(fmtval)); // check we found the time properly - assertTrue("Format came out incorrect - " + fmt + ": " + fmtval + ", but expected to find '11:23'", - fmtval.indexOf("11:23") > -1); + assertTrue("Format came out incorrect - " + fmt + " - found " + fmtval + + ", but expected to find '11:23'", fmtval.indexOf("11:23") > -1); } // test number formats @@ -358,8 +358,10 @@ public final class TestHSSFDataFormatter { Cell cell = it.next(); cell.setCellValue(cell.getNumericCellValue() * Math.random() / 1000000 - 1000); log(formatter.formatCellValue(cell)); - assertTrue(formatter.formatCellValue(cell).startsWith("Balance ")); - assertTrue(formatter.formatCellValue(cell).endsWith(" USD")); + + String formatted = formatter.formatCellValue(cell); + assertTrue("Doesn't start with Balance: " + formatted, formatted.startsWith("Balance ")); + assertTrue("Doesn't end with USD: " + formatted, formatted.endsWith(" USD")); } } diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestDataFormat.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestDataFormat.java index 79b14e953..bbdc49935 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestDataFormat.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestDataFormat.java @@ -87,4 +87,48 @@ public abstract class BaseTestDataFormat extends TestCase { assertEquals(poundFmtIdx, dataFormat.getFormat(poundFmt)); assertEquals(poundFmt, dataFormat.getFormat(poundFmtIdx)); } + + public abstract void test58532(); + public void doTest58532Core(Workbook wb) { + Sheet s = wb.getSheetAt(0); + DataFormatter fmt = new DataFormatter(); + FormulaEvaluator eval = wb.getCreationHelper().createFormulaEvaluator(); + + // Column A is the raw values + // Column B is the ##/#K/#M values + // Column C is strings of what they should look like + // Column D is the #.##/#.#K/#.#M values + // Column E is strings of what they should look like + + String formatKMWhole = "[>999999]#,,\"M\";[>999]#,\"K\";#"; + String formatKM3dp = "[>999999]#.000,,\"M\";[>999]#.000,\"K\";#.000"; + + // Check the formats are as expected + Row headers = s.getRow(0); + assertNotNull(headers); + assertEquals(formatKMWhole, headers.getCell(1).getStringCellValue()); + assertEquals(formatKM3dp, headers.getCell(3).getStringCellValue()); + + Row r2 = s.getRow(1); + assertNotNull(r2); + assertEquals(formatKMWhole, r2.getCell(1).getCellStyle().getDataFormatString()); + assertEquals(formatKM3dp, r2.getCell(3).getCellStyle().getDataFormatString()); + + // For all of the contents rows, check that DataFormatter is able + // to format the cells to the same value as the one next to it + for (int rn=1; rn