From 509c7bcb931ac319a52c3c900fffcec03a12079b Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Tue, 3 Nov 2009 22:45:39 +0000 Subject: [PATCH] Hopefully fix #45672 properly - improve handling by MissingRecordAwareHSSFListener of records that cover multiple cells (MulBlankRecord and MulRKRecord) git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@832584 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/status.xml | 1 + .../MissingRecordAwareHSSFListener.java | 42 +++++++++++++++++- .../poi/hssf/record/MulBlankRecord.java | 7 +++ .../apache/poi/hssf/record/RecordFactory.java | 17 ++++++- .../TestMissingRecordAwareHSSFListener.java | 39 ++++++++++++++++ test-data/spreadsheet/45672.xls | Bin 0 -> 17408 bytes 6 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 test-data/spreadsheet/45672.xls diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 0cd747f15d..2d9fe5a3dd 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 45672 - improve handling by MissingRecordAwareHSSFListener of records that cover multiple cells (MulBlankRecord and MulRKRecord) 48096 - relaxed validation check in RecalcIdRecord 48085 - improved error checking in BlockAllocationTableReader to trap unreasonable field values 47924 - fixed logic for matching cells and comments in HSSFCell.getCellComment() diff --git a/src/java/org/apache/poi/hssf/eventusermodel/MissingRecordAwareHSSFListener.java b/src/java/org/apache/poi/hssf/eventusermodel/MissingRecordAwareHSSFListener.java index e41b0b92ae..7da8ee5e20 100644 --- a/src/java/org/apache/poi/hssf/eventusermodel/MissingRecordAwareHSSFListener.java +++ b/src/java/org/apache/poi/hssf/eventusermodel/MissingRecordAwareHSSFListener.java @@ -21,9 +21,14 @@ import org.apache.poi.hssf.eventusermodel.dummyrecord.LastCellOfRowDummyRecord; import org.apache.poi.hssf.eventusermodel.dummyrecord.MissingCellDummyRecord; import org.apache.poi.hssf.eventusermodel.dummyrecord.MissingRowDummyRecord; import org.apache.poi.hssf.record.BOFRecord; +import org.apache.poi.hssf.record.BlankRecord; import org.apache.poi.hssf.record.CellValueRecordInterface; +import org.apache.poi.hssf.record.MulBlankRecord; +import org.apache.poi.hssf.record.MulRKRecord; import org.apache.poi.hssf.record.NoteRecord; +import org.apache.poi.hssf.record.NumberRecord; import org.apache.poi.hssf.record.Record; +import org.apache.poi.hssf.record.RecordFactory; import org.apache.poi.hssf.record.RowRecord; import org.apache.poi.hssf.record.SharedFormulaRecord; @@ -62,7 +67,7 @@ public final class MissingRecordAwareHSSFListener implements HSSFListener { public void processRecord(Record record) { int thisRow; int thisColumn; - + CellValueRecordInterface[] expandedRecords = null; if (record instanceof CellValueRecordInterface) { CellValueRecordInterface valueRec = (CellValueRecordInterface) record; @@ -105,6 +110,19 @@ public final class MissingRecordAwareHSSFListener implements HSSFListener { // - so don't fire off the LastCellOfRowDummyRecord yet childListener.processRecord(record); return; + case MulBlankRecord.sid: + // These appear in the middle of the cell records, to + // specify that the next bunch are empty but styled + // Expand this out into multiple blank cells + MulBlankRecord mbr = (MulBlankRecord)record; + expandedRecords = RecordFactory.convertBlankRecords(mbr); + break; + case MulRKRecord.sid: + // This is multiple consecutive number cells in one record + // Exand this out into multiple regular number cells + MulRKRecord mrk = (MulRKRecord)record; + expandedRecords = RecordFactory.convertRKRecords(mrk); + break; case NoteRecord.sid: NoteRecord nrec = (NoteRecord) record; thisRow = nrec.getRow(); @@ -112,6 +130,13 @@ public final class MissingRecordAwareHSSFListener implements HSSFListener { break; } } + + // First part of expanded record handling + if(expandedRecords != null && expandedRecords.length > 0) { + thisRow = expandedRecords[0].getRow(); + thisColumn = expandedRecords[0].getColumn(); + } + // If we're on cells, and this cell isn't in the same // row as the last one, then fire the // dummy end-of-row records @@ -148,13 +173,26 @@ public final class MissingRecordAwareHSSFListener implements HSSFListener { } } + // Next part of expanded record handling + if(expandedRecords != null && expandedRecords.length > 0) { + thisColumn = expandedRecords[expandedRecords.length-1].getColumn(); + } + + // Update cell and row counts as needed if(thisColumn != -1) { lastCellColumn = thisColumn; lastCellRow = thisRow; } - childListener.processRecord(record); + // Pass along the record(s) + if(expandedRecords != null && expandedRecords.length > 0) { + for(CellValueRecordInterface r : expandedRecords) { + childListener.processRecord((Record)r); + } + } else { + childListener.processRecord(record); + } } private void resetCounts() { diff --git a/src/java/org/apache/poi/hssf/record/MulBlankRecord.java b/src/java/org/apache/poi/hssf/record/MulBlankRecord.java index 87d4eafeac..695c55c877 100644 --- a/src/java/org/apache/poi/hssf/record/MulBlankRecord.java +++ b/src/java/org/apache/poi/hssf/record/MulBlankRecord.java @@ -56,6 +56,13 @@ public final class MulBlankRecord extends StandardRecord { public int getFirstColumn() { return _firstCol; } + + /** + * @return ending column (last cell this holds in the row). Zero based + */ + public int getLastColumn() { + return _lastCol; + } /** * get the number of columns this contains (last-first +1) diff --git a/src/java/org/apache/poi/hssf/record/RecordFactory.java b/src/java/org/apache/poi/hssf/record/RecordFactory.java index 3d200e7ef4..85f3a082e4 100644 --- a/src/java/org/apache/poi/hssf/record/RecordFactory.java +++ b/src/java/org/apache/poi/hssf/record/RecordFactory.java @@ -281,7 +281,6 @@ public final class RecordFactory { * Converts a {@link MulRKRecord} into an equivalent array of {@link NumberRecord}s */ public static NumberRecord[] convertRKRecords(MulRKRecord mrk) { - NumberRecord[] mulRecs = new NumberRecord[mrk.getNumColumns()]; for (int k = 0; k < mrk.getNumColumns(); k++) { NumberRecord nr = new NumberRecord(); @@ -295,6 +294,22 @@ public final class RecordFactory { return mulRecs; } + /** + * Converts a {@link MulBlankRecord} into an equivalent array of {@link BlankRecord}s + */ + public static BlankRecord[] convertBlankRecords(MulBlankRecord mbk) { + BlankRecord[] mulRecs = new BlankRecord[mbk.getNumColumns()]; + for (int k = 0; k < mbk.getNumColumns(); k++) { + BlankRecord br = new BlankRecord(); + + br.setColumn((short) (k + mbk.getFirstColumn())); + br.setRow(mbk.getRow()); + br.setXFIndex(mbk.getXFAt(k)); + mulRecs[k] = br; + } + return mulRecs; + } + /** * @return an array of all the SIDS for all known records */ diff --git a/src/testcases/org/apache/poi/hssf/eventusermodel/TestMissingRecordAwareHSSFListener.java b/src/testcases/org/apache/poi/hssf/eventusermodel/TestMissingRecordAwareHSSFListener.java index aa4bbcc6f0..e632cbbbb1 100644 --- a/src/testcases/org/apache/poi/hssf/eventusermodel/TestMissingRecordAwareHSSFListener.java +++ b/src/testcases/org/apache/poi/hssf/eventusermodel/TestMissingRecordAwareHSSFListener.java @@ -29,7 +29,9 @@ import org.apache.poi.hssf.eventusermodel.dummyrecord.LastCellOfRowDummyRecord; import org.apache.poi.hssf.eventusermodel.dummyrecord.MissingCellDummyRecord; import org.apache.poi.hssf.eventusermodel.dummyrecord.MissingRowDummyRecord; import org.apache.poi.hssf.record.BOFRecord; +import org.apache.poi.hssf.record.BlankRecord; import org.apache.poi.hssf.record.LabelSSTRecord; +import org.apache.poi.hssf.record.MulBlankRecord; import org.apache.poi.hssf.record.Record; import org.apache.poi.hssf.record.RowRecord; import org.apache.poi.hssf.record.SharedFormulaRecord; @@ -422,4 +424,41 @@ public final class TestMissingRecordAwareHSSFListener extends TestCase { assertEquals(1, eorCount); assertEquals(1, sfrCount); } + + /** + * MulBlank records hold multiple blank cells. Check we + * can handle them correctly. + */ + public void testMulBlankHandling() { + readRecords("45672.xls"); + + // Check that we don't have any MulBlankRecords, but do + // have lots of BlankRecords + Record[] rr = r; + int eorCount=0; + int mbrCount=0; + int brCount=0; + for (int i = 0; i < rr.length; i++) { + Record record = rr[i]; + if (record instanceof MulBlankRecord) { + mbrCount++; + } + if (record instanceof BlankRecord) { + brCount++; + } + if (record instanceof LastCellOfRowDummyRecord) { + eorCount++; + } + } + if (mbrCount > 0) { + throw new AssertionFailedError("Identified bug 45672"); + } + if (brCount < 20) { + throw new AssertionFailedError("Identified bug 45672"); + } + if (eorCount != 2) { + throw new AssertionFailedError("Identified bug 45672"); + } + assertEquals(2, eorCount); + } } diff --git a/test-data/spreadsheet/45672.xls b/test-data/spreadsheet/45672.xls new file mode 100644 index 0000000000000000000000000000000000000000..752ce5da02cc04d07e825f4e8588cb1058604fd2 GIT binary patch literal 17408 zcmeHOeQ;dWbw6*ll2#wqw`JL~R*ODt*_MPX$-<6fuWZ?T4D!c-gJCR>kfoI^tPfXS zCH54LMbpeMg{F>NAhDIC)X+ls7Q}=pAA&!inUPZ_1NDHR;=NBS`59?tBW2Jhi5$vO;0Erw zic&;Wg&ThUB#}r+AQ5iAD4!td~N&w9wA-)pLiYr|yzvOjtSH~?A!5RJr%}^=m=FUA-~TDd_%MTx-O&09S*a7H!@Xkk!ak{|x%X@e}M)luTn2qGAY< zFid}JY>?`R<+4Jla)a=IH#VuWYg=z@m<}L%*V40W$^dQ7HoDN)5sXIJ>(vydMi+`< z*hJDWMQ~nEZo|PUtR%1o zumJ1#To)VIs}h8<%)kP{B4}<=+v6!5tc7q)7b=CuMUbtbmDEDi%feSigeJXBQWyKt z#MU&`0N2~p+1nHjHqr`O&JAs{9_60l4*Ct$h(LA-f&!W>39-dRqqe|yT1A&D08ZwyhNhY*_Udpjio|0BP)3nyIt*~Mv{g4NYX2q7?2u7O@?1_YfH_=&)&7{QD z7(Fet4$A1tI*rk!DDKrF99&LMW70^2`9<+=Li9WF-oZ0Fm)Quzd8!xi4+L;tI8T*s z^j>i0>PG*0h8u`HyGT3po1n16Qzj<(ePRDp_#+5T@Uyj z9&j&xy!byU_+X~g?DIVjxYQ?=Uh0zyzt4lu6CUuVJmAlJ!2ikv{)`9w)9Uj>!H0Jp zobe;$gm)hV{BMhk@5#F-kAim#dfttY@WH_ss$Zz4;L`$bU7xuSvi)zwi!}fNT2m4wHbthF2Ean6wY?K)KNq))gWziPv8cnfenJ z3~umj0k4(YR!nZvXJgqR=-J?+NIQ@PSNPu=Dx+K35XrKYI{y64;IYsqI_7}0)ibc1 zDHA&hJSUk8h4lqkVF!W&5gTOQa*?na40I-J+BBGOBziO9@stJ=4j*qO94u)t$@XPZ zmL3zfqqim;4ryq@QRdBr$Mn2R=DPj8!vUKH6Mye;c29$ezjt`bO@oQQcX(Gf4JQ8H z;cekGnD~2#_myoXcyJp#t!@6^;U6>7VB+r`-q}opiNAM{$5tW`|rO$MX6{snxYiLbfp+(vc@a9?2rmO7HZNg zbveibNlS6(9p+A*(y`}b20kl`m5#FkuL~IQ@^4lD=<5RP#3-Xjm_s!M+ns^_39>C6 zCW)v&llF9&=&4#aBL8$FjmTQ3bSE_wt^k=GpV8=7-Hn)-`%X$I{Qt$L?v%AjQ?LV5 z28}fs5|c(!snqaT=U_4&^9YZ`ucj3yN3@{CULV$&RwyMBb4l}9qjY9wCP5|>OECd6 z&0~$yBn%_P8W2esq@=oLJ)o*xN_%JRj6Apr_yQ#FYuHQ^vZvT>T{WS|7wba;bo zZK;nmMK<;K$429;r$XCkisufq7{PF20d^u=s2=+=O{kmji(@}_9{CqvnPCj0jEb^qd-AGwRE(8ah!+HxN;YNX9|pMU3BcQK1~F)op|%14YEX>;9kvv0bKS)z+^ ziL}do#Hf)r*M0Ww_uR!)>SA0Xt<6V_8fkOgAHDvRyO^acro%tdA`sJ&Mx?d2-Sy;o zDF$KAXC#t2tx6Z;5@~flV$?`$ZF}RMyWGWubuliH*5D&XjkMObm;UvXyO?TSj7y|7 z`iM~@t+j3D-dEhkM07DOk=EiPMvb)Awv*?~46Xa-GF^;Iq_z8qQ6sIj?ZepGib&9z zuGW~U-Y5F+yRUR(8lnbXf^F^_mwp4@SCT!^2adGBY5g3#Z}a zX*|dxrfrwFxFdXfuJcP2*zi+i#wAWGT&) z*=W(Mwa8B7r?ha&T8Pse*t_3)6XkUD6{AgrX>_~hnbd{4;U`?V3M=X!vC>{HL zRQR8dJ|2b5cR`&ahzEY_u)NRCHp44OEt5IW?n@fWoJ0u@SdIyIjnUV@WB|;r2FpXZ z9|wF0f)1Ki$-zn=FXvZ{jUn9EJ6Rb^5<$8Otj4r}=Y0iBBdBa?RDib$>?>Uz>l+vv z-5FBn-RiYmP~F9MIX&Os3P5fdXn%nInVh%!^%^ z%Pi`_JQ00h_TSJEX5OwcM<&&om$)#O+0=u%TwnwQ^EE1SWK=D!WR$uv=d3FIo~@dN zqq1W%lp6bikYM8OGtae}G8M>(-DZf&(6w_<#gbnH)6LI0-uW3RTg7ps3TT`@kpz+|83?e$RksJPpPzH`3h6Y1+<#Uc8th#;GS z2h4s<*Sv4jK{t!j%iq8Z^lCF(IYd{EjmP@?4#!W0D(I9V)nGIVf1kmb?=uRH;ddgC z4V+PX)ZlL;xCr?g-g2C2m*>mv>x)K+UIfFcUm5ff{@uabSzNJIRo8-XaR3E8hPWiK z4hhjNXbB5bf2bT4H)bg}zo+PK=ytb)+5E0SrvW>yz_<^00(PeYV+MR;LY`+Ya-3(+ z3zRWh6y8Sr1$N=PvFuy`O?gxXVDmz8U(CA0nj!k=!S{cv=fDdi3O*0hz=c8^pAVTTzXdq0e>T^aUy9Sj7xHcTGHjEMDEchK zzl)}n=S4UJJ)+Q8;KcET1$O(ze85=2FTuf;3rhP+{4+`I&r-zkSw-I}{L4+{7v|&0 z3jb;x%o!^nQ6bcx7Q-ysjVwVr?Fy?vjkEsB4nx8Fr z6X`NPdGwt4Ulp%Qsc57V*Ml>-E~t-eDxeIZK_PI1Tsp=(pJROy(OvXPh>R}Ux_ir} zE%eOaZ7{E;BxRRD2AAisj^_C7qS=`jz?W`$57(y~Y5^(g_$fm)sg0+Kl^Zz+zTkH{pHT`s5+@DuK-hM3m0`o#+HD_yn^lgP+9_a~>f1#gtV_ zjrjM?>|*$$`qX#0{4$y;+b^)%A9+NyFTg}0+Dmi+(0S+29NKXE;{wf7k%BG+`o2RO za?jU}2y~GXU1YX?d*4NYE_R}eE#|Mi*4Y46OPuHupuas@^7Yvpej?DN5?zw)T1hn( zB)b+Y(z{lwwdVyMbT2V2R|d3ISo%UIdLhu~=1}%LBczu*(dAa_+y7ml7dg?3fc~48 ze%H`LGXh=VL|2&1yKB!0G*8cpYKtxAa4M^|#ED*Fq2V(|S31#^KwCaz^in5!DbSYB z7+vK=SDCHZXN(Rz(P4|Z>9cAlx*BNPXR|~fn;kn(A^J@Q{}1xlv=Kj^ zWZ_U-{5O;5d{&#!FYx~~`SbvPe^EZ47Wf1cQ>Iu&!qH0MxVZR@?_c`EjsryxeJez( zmOuFh&kT>Fa%?cZ6P4E(SEF)_-i(Uy#a4z~<(Rdl>;bfW#_ty@m_<3FYx&*>Du{s&$^_xf~m zWzord(Er9iJj(rl6_v*X_kSlUpMJd&l@IqFLd8q3=zkHuCsDcIx1;h3_XH~2@f4ln)>aU}LjlyT`^49zGC@ z#SaU