From aad9e6907d4ef1897ef8e05917ca476728f8e8e0 Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Thu, 20 Jan 2011 13:20:24 +0000 Subject: [PATCH] Fix bug #49928 for HSSF too - refactor HSSFDataFormat to allow overriding of built in formats, and tweak the format unit tests to do the same check for HSSF and XSSF. Also corrects some builtin formats for new HSSFWorkbooks which were slightly off git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1061288 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/status.xml | 1 + .../poi/hssf/model/InternalWorkbook.java | 18 ++--- .../poi/hssf/usermodel/HSSFDataFormat.java | 63 +++++++++++------- .../xssf/usermodel/TestXSSFDataFormat.java | 25 ++----- .../hssf/usermodel/TestHSSFDataFormat.java | 21 ++++++ .../poi/ss/usermodel/BaseTestDataFormat.java | 33 +++++++++ test-data/spreadsheet/49928.xls | Bin 0 -> 16896 bytes 7 files changed, 109 insertions(+), 52 deletions(-) create mode 100644 test-data/spreadsheet/49928.xls 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 0000000000000000000000000000000000000000..d7249e7bdaffd959d9b745ae8e2728038b237363 GIT binary patch literal 16896 zcmeHOdvH|M8UOC?Cb=O9350+=!W!O?MA!`o@>m`sI)=vx4tB)BknBPvCUjja%t(o? z{!^)gi9!o$t+mcrwLZ}Lm}-Uop?|oxomT5;wSygXto9|f)9R?(-*?Wvd(S<4@7>+1 zo#JpZ=jNX8p7Z;@^PTT}=efWBRo%%uA0Gb(X<-?K>C;>_RVs7??;J&_C7OsAPJf!q ztr-u7%_rafyixMg=ivHBMw8XK^%^F7UBrRk%%w>szV%wI2y4Y zaSUPu;#kCSh-V{?N1TAjcEYhyc|VO*^`#W`QtHEBhN85ZdhtI^d(k?jhXz=Tt(Xvs zMNNJZ9T!i!Q2vw8=)sej3xDvuF$u}zG8=6M_cZOKZ9uW9LWZPh zf%4>82>y26OXbh{swTWI_SwZ(sexd3Fym=))zCj^+C~^W%qCX;TpWk)BQZ&c1A~_67 zX!_74HK?sf4nmIL);BONy0xxz;ZZXfBZM(BIEMi%EXJkU7o#yU4!#-Va?@Q+Q<|qt znY(yPi~ZUdnS@auy$ew<~>CJ%tg@)>~K7KhR(Y9{Pkc^)K)UQp2X)_ ztI+?W)~CoFLjQ&IpZL)4^`SrDLqF<6f6#}1zYqO;KJ=S?=zj9};Xfj9cvkM#&-Z=k zQl7&6Ql3Kkk9=?*_Mt!OL;txC{aZftCw=JU%JWl!!>b#&ex#l7Do4<7tE)R)bGU{g zheUo}^+PtGW_AGi7uc*F8*ZA^kP~^|+N7_HH3@x0i@QMYX z#NidBpy%bh&(fPkAF8WssA+(nj|e)iG;MsTXQ}@RjK6}e>6urqg09I~Z_{7IRF0PA z^}s7vul&3Q7Ji1O${gTlt~cxHdRAOMBLxQxz79p#J>f*}wfV=U@MsE|4Lr^aja27X)&i8zku?N$$NNMUq>!=9~|Z zD$$%Qbs}+{k0jae=0U2gD(>IEKgZ3>ft#HHR9dw{!zU4jdC>Nmx=@RdR~0JZE!3$~ zrwW8hBoYNesWe+Crdcv!%`AmiX@x?~atZa(J|A^^38iY~vuR52jSC=@>;tU*0-h#@;B*9>jtwt>Q0U=eN^ z4cDSz{$qE{TrOKsi(|)*6&Q_5vunXLV2fZIjY=z=V_{*h)Pn86rP_VwWmz%8oIyKh2O_ROR%jGtql;bUtC~RaK#NTi64BG)IkdAd6resQ$uF!+>K}}JR zYj7PJ%(El>#D9Fa4{-t7u0O zU!CxJ!ykkXHj9E}+k-){5LG~s@YYE%$n;V`uly1vS|%q!^;JWjfuAT z>p#5NjM3RFC}a~0U?ZosqBhmZ)F*#>#hXn7vuO*qw8>!8R)VEv+n>n3>CI-W&c?&i zrUbB2EiK#r_LEO|vl*we@vyWR0c=!D%eKG#=2PBm&eqv@SlXNbHmap%+q1`C^=30( zXX9aM=LWD*EiK#r*z50jvzef?@vyXb02|fPvh5$g^r$zRiOgm}u%$)8WiswON+&i-T9n1o2bsl!_wvkuu&~779V`U^3ZzQoTIbxu(Y-SHmaq?;vXG9=vABH z)PzMW<;Tqx+PFC>SLHia60N`CX>ZUe4k#>22R%mvRjmkTC>z##gHCloVLv+PG!0a> z8KR><`-(T{GzS#cqJz%TKvfGN>hJoYH|TT+6tHSow%VF#d{Lc z^`P30RJ;`>CWUGzwWZ?yr^>ukGG$98Q?^txHHb~yIsM_>fEjg*QV|A|Rk$c8FY198 zao*GhHha$(bm#8yCFNV(!~%U4vvUt^HLf z(W;fG$_+0l;pR1l&gCnztM}~c>FDjq^riPkHzoIE2Fx{XsL=deEpA`#!7V#(`(Dxh zd@FJ|k^2SZ)##x=(0BD4ld>(fk?cf@ThFtf2n z;KCSI5Emsoy1IL}N3mAW1P#|hvll@iw*77hx*Y<_HG(h!bePE(?~`zBidB$II>N!w z2rM!baQHqQyv_q$`ad7=T;iVNe?UbTJg$Pn2kPLXJiw(N^Z}PsgOC7kQ^Da2HHQ!$ z?E%jIP_|f=s+PgfFq)5sq>FoZ?S?7rb~jn59`)fXsJmc`+~`h6%VFQ&ZZtj|_>A4Z z784}+rS|ULoyjh%|6igBGY)>KT{QGgGN+@nPod@}8U2=CO z-2wNaIc8%b9P>H=b4}B*DRwM!MI-zv4omF%4&g+_GI*F&KcPFu-Le`4mg`{YX>pU6 zp)Bnicy$)5)wm%Bh;@CL=-5n)uW{RP&$?MGy7?G&3>9W6y0(dLeHFS{)1UK4IXLpAW!o53NC zer0=D4Kz*TtHWeyT&)6R$KT;8J4keM>3h2$COcgqG?lNO!*FbSDO+)VveIDid@pP z9UGfgks`5VSc`yw~B$ zzcDfC;~&5Oo3-0EB(@6rY(bwRp5G9Zex**+VC6*3#try#BEr1JAbwNizEM1%K zw-Ymz_nCQ=cM`Ld_dBr=U_=A*8PF(eAuhsi6ZZcM4Pi<#r0D>~@?i;&HX>XGDUcC^ z76;je{4_&+ro)tGi#U?Ljt<0uZ&3U_@iHudWkDLdZjbR!IW6ah6>dg5-0YGeM3bOR z3_u(tD$#axETW|G6N=}hZ}@_sR+F;Xg{HzTPoEa!3fe7Y|HmZ#zicsE2UaVHg z^;8NAKqD)G?c(H3cbxig{f^rEZZ&Dnj7MJK>B&Qgd>qN&mm~6Mdl4er%2kM?5ceQr zRz?RAv001l5c&Jid+?ua?M)Qo(_kJ`)E|{0S(LuEyEEN)ZC@%AUA?C>*&}4=KXBRA z6E{6?EJykN@Ew0yjo%nrV;(~WhM@X~UV7oBq1FktgNIT7<_{m>`kzGPV>Yh;2E>tw zmm~7E0bX_R1y5eMaow&*9F2G*Vgup+tMAqU3iyu7%ni8 z^!?umW}KJXMb0Lj*O>Dfr{T!R3|aXs=;prk&TD!4d95g4s?Ja{9}IYgFdPK;4E?vC zd@${_*ZNedyE7@9EgG9UHy+Ki7anJ2cPh0Ib6(ytLCdy^nzSyY4;CH!YM$Mhm(N)P qhP73;3yaWgbamP2{0mIgE0zMl{U=>Y#uD?C%fCd}lCz!8{Qm=0KR6x$ literal 0 HcmV?d00001 -- 2.39.5