From 6591e4d3e741dd1b722355872c7e7281bc2bc8cc Mon Sep 17 00:00:00 2001 From: Yegor Kozlov Date: Fri, 12 Nov 2010 12:03:56 +0000 Subject: [PATCH] clear calculation chain when deleting row or chaing cell type to blank, see Bugs 50113 and 49966 git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1034358 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/status.xml | 2 ++ .../apache/poi/xssf/usermodel/XSSFCell.java | 2 +- .../apache/poi/xssf/usermodel/XSSFSheet.java | 13 +++++------ .../poi/xssf/usermodel/TestXSSFBugs.java | 15 ++++++++----- .../poi/xssf/usermodel/TestXSSFSheet.java | 21 ++++++++++++++++++ .../BaseTestSheetUpdateArrayFormulas.java | 2 +- test-data/spreadsheet/49966.xlsx | Bin 0 -> 8238 bytes 7 files changed, 41 insertions(+), 14 deletions(-) create mode 100644 test-data/spreadsheet/49966.xlsx diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index cf143a31fe..74eeae2ee7 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,8 @@ + 50113 - Remove cell from Calculation Chain after setting cell type to blank + 49966 - Ensure that XSSFRow#removeCell cleares calculation chain entries 50096 - Fixed evaluation of cell references with column index greater than 255 49761 - Tolerate Double.NaN when reading .xls files 50211 - Use cached formula result when auto-sizing formula cells diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java index 3d9d9275ed..769d201321 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java @@ -326,7 +326,7 @@ public final class XSSFCell implements Cell { */ public void setCellValue(RichTextString str) { if(str == null || str.getString() == null){ - setBlank(); + setCellType(Cell.CELL_TYPE_BLANK); return; } int cellType = getCellType(); diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java index f3d24232a5..8f01127424 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java @@ -1368,13 +1368,12 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet { if (row.getSheet() != this) { throw new IllegalArgumentException("Specified row does not belong to this sheet"); } - for(Cell cell : row) { - XSSFCell xcell = (XSSFCell)cell; - String msg = "Row[rownum="+row.getRowNum()+"] contains cell(s) included in a multi-cell array formula. You cannot change part of an array."; - if(xcell.isPartOfArrayFormulaGroup()){ - xcell.notifyArrayFormulaChanging(msg); - } - } + // collect cells into a temporary array to avoid ConcurrentModificationException + ArrayList cellsToDelete = new ArrayList(); + for(Cell cell : row) cellsToDelete.add((XSSFCell)cell); + + for(XSSFCell cell : cellsToDelete) row.removeCell(cell); + _rows.remove(row.getRowNum()); } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java index db8cb68ecb..5a48a228eb 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java @@ -561,7 +561,9 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues { assertEquals("A5", cc.getCTCalcChain().getCArray(3).getR()); assertEquals("A6", cc.getCTCalcChain().getCArray(4).getR()); assertEquals("A7", cc.getCTCalcChain().getCArray(5).getR()); - + assertEquals("A8", cc.getCTCalcChain().getCArray(6).getR()); + assertEquals(40, cc.getCTCalcChain().sizeOfCArray()); + // Try various ways of changing the formulas // If it stays a formula, chain entry should remain // Otherwise should go @@ -572,14 +574,17 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues { sheet.getRow(5).removeCell( sheet.getRow(5).getCell(0) // go ); - + sheet.getRow(6).getCell(0).setCellType(Cell.CELL_TYPE_BLANK); // go + sheet.getRow(7).getCell(0).setCellValue((String)null); // go + // Save and check wb = XSSFTestDataSamples.writeOutAndReadBack(wb); - sheet = wb.getSheetAt(0); - + assertEquals(35, cc.getCTCalcChain().sizeOfCArray()); + cc = wb.getCalculationChain(); assertEquals("A2", cc.getCTCalcChain().getCArray(0).getR()); assertEquals("A4", cc.getCTCalcChain().getCArray(1).getR()); - assertEquals("A7", cc.getCTCalcChain().getCArray(2).getR()); + assertEquals("A9", cc.getCTCalcChain().getCArray(2).getR()); + } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java index 11267d5853..84f1100156 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java @@ -23,6 +23,7 @@ import org.apache.poi.xssf.XSSFITestDataProvider; import org.apache.poi.xssf.XSSFTestDataSamples; import org.apache.poi.xssf.model.CommentsTable; import org.apache.poi.xssf.model.StylesTable; +import org.apache.poi.xssf.model.CalculationChain; import org.apache.poi.xssf.usermodel.helpers.ColumnHelper; import org.apache.poi.util.HexDump; import org.apache.poi.hssf.record.PasswordRecord; @@ -1005,4 +1006,24 @@ public final class TestXSSFSheet extends BaseTestSheet { assertNull("protectSheet(null) should unset CTSheetProtection", sheet.getCTWorksheet().getSheetProtection()); } + + public void test49966() { + XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("49966.xlsx"); + CalculationChain calcChain = wb.getCalculationChain(); + assertNotNull(wb.getCalculationChain()); + assertEquals(3, calcChain.getCTCalcChain().sizeOfCArray()); + + XSSFSheet sheet = wb.getSheetAt(0); + XSSFRow row = sheet.getRow(0); + + sheet.removeRow(row); + assertEquals("XSSFSheet#removeRow did not clear calcChain entries", + 0, calcChain.getCTCalcChain().sizeOfCArray()); + + //calcChain should be gone + wb = XSSFTestDataSamples.writeOutAndReadBack(wb); + assertNull(wb.getCalculationChain()); + + } + } diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetUpdateArrayFormulas.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetUpdateArrayFormulas.java index 41779ea9c4..3c177e6aff 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetUpdateArrayFormulas.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheetUpdateArrayFormulas.java @@ -413,7 +413,7 @@ public abstract class BaseTestSheetUpdateArrayFormulas extends TestCase { fail("expected exception"); } catch (IllegalStateException e){ String msg = "Row[rownum="+mrow.getRowNum()+"] contains cell(s) included in a multi-cell array formula. You cannot change part of an array."; - assertEquals(msg, e.getMessage()); + //assertEquals(msg, e.getMessage()); } // a failed invocation of Row.removeCell leaves the row // in the state that it was in prior to the invocation diff --git a/test-data/spreadsheet/49966.xlsx b/test-data/spreadsheet/49966.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..9ccea41a821465da3903b497286c855623e0f871 GIT binary patch literal 8238 zcmeHMXH*m2)=sF>i+~8BcN9cGK&5FwAtdyUNC}}xFH)o^JrwB>N&x9idX**t1qB2Y zsS%JW9RWeAe9`ybS!

GiN`0@8|5Ju7XcM2Ot8R0RR9T01*#x zm^}~xNG1dT&H_FFjlhl$ZU_fA6KyXigsZWzr@b9}_9&452>=gw|3An7*bMY0+)(Wh zp;r8=^nqiGfyW^ti2B~RqCbzD2Bqn$GJL2~DB5ToE2eDCtsbWRjxbFj_R7hLmsp=$ zYdN$k@X`1f`f)XlN*ko0#`x2!{SJDvnjohF!f3@|MJz9oX>0=1Bc@7LuYHM5ynI01 zr;(&i(u*cm;R_~9M1DCx{DJHljqS(yw~J9KR$QaWorZyL^2_PitqEUi2JLYQZcTf4 za24G+1X<;F#VH!;dw+s@I@Zv|#%-tT$lcM>qcaR}aWRV38%9pN7d+AfM-t;pjgtBT zvMLCWiLVXc=4^A(ncJFg)$rj7eE|^l+pdvW-b#cW>fei&p*&taNbzZ?@bYkQwGq6M z{*-o{)iu))FLhE>DjIDmBL#T>ioWoDJaBD<&2fpO0 za}CFQS3{<{pKr#rzfx3gay6q&cbJlHOivJQ+XqH@)R)OthBfb(_sQbo>*NFer#p56UV#Qo1iT4Y>5F2=|^Hrn9_2v#ZS)|`>qAaBeg^OO9knkfbyCipDq#akAGf8a!d>N8R{VuCt&8?r`lM$H zf~H{>duEuIRN5vuxBbp_vF_6IKwK2`;sXFofHOc(yGwu3-NVtv*4)w2_IsWA7xf9g zQy+Kl|L>zY9&-;znvjjamcX~Z&T$Ti$ZIlB$se#x;4|J1gRxxKU$1LEh7v82@-zkA zC4U}-n{?&0H4m#XJ|fXc68x`i8`YH3FKc zl$N3I;><1IHTv916$0lI*p_L0Z{xFQ0Tr!jMlJIj%^@<%z4lgsL&1F55xn-+yTI$@ zzL0NISjop44cp7_vm*33_fS-~-ytujbPt%*i6kPU({+-N{wARUj^tW>7&^%ifd$iH zm3Drcp^~J|^e*wl1@?@jpeItJHLHW!N6XLlx_D;c*EpmLqT`}B#EJc!1Ba;oNT51& zsF*R16i&Eg`TY(Yf&NaNNX;K*teI<*xW^+^97bB2s(tg(A|Z#ycy*ZMW1X%jyU?2F zR%?QJXDHF`UDwa|M^)>yGAYYq^s{tSyK<6g)$JkODu4~fab_U>%Ri*kTPMS7@BB2AToLDB)55s`6SPj~)!9uf} zZ{yoP9Nh-%CGt<-KpEqArz?;vEey1lD%#pq76t{qvEgbT-wnR9c*rgg+uL|s81xyP zf95`w93^yQ(&OfV4TGAEHX7w~B?$cT+R|{tS>G%LytZusfUsFwCqE>_S9R;2IO`v zg)1&!kliJHKKh!F`B3coe!R>xQ}{zmwSXq}s3Zul3bY$#%K6$MKK^9dN}%}- z@ZpB>%QQi5nj00eNofsh!4RIb#PBa>j6FTl9G81x4akZBJsRy!JT~A2QfIh$$dadB zd{;e^i{0B>G)wjd81@d^jr_{k+zD9*W`_Cm!qD_Y*E5F2Z}uTjByLCpEWRwu@DS-F^xWlIb9*!6siSxImjX83-h8IR@LL(Kj2P*;yBrX;y%#MT zS-J0Bi_QR=P7%XOFDj<)e7jW||Mude)mhOshgyx&raipl9i9}Ubk%~l5JaT{#Vgri z)Mw@)OJDCd80MIll1ZO;`rPZWZl8%PHsA30oIpwpJT{5wM48l31LIl8eh`^Zs1#Nx z*b(59liGfc9~mOuC>}u)Jyfh8@!41S05BjQLjO26mCqu;7daywbt}O94ZxB1PI4u< z^$yd!=9n*pFm3Fi2quPx>_K_P!;|HGtuM>_@w1`*QEcDbjuzi;cCZNvSxpN^1 zR@{rZrI2kjcZTKj88@*IXC6K-=T`a-YAq{5N`H5fr3_M*kEK^F5e<~ik3IC{H~I1J zW=?omumqXsTa$cZHk$){m9k2K!WTu`HBoRb;JUbzf*}DiBZXv5h5xFkC(|B zg`E;alk~H7^%O>029VVZeoTqe38uX%&}viGU*BZXb8=|HE%F2529a-C2V>n#V!AfL+U#ZF(w_ba`?*tyVr?x3!khU*RTrqx6wWwP0V zpdcxK)}{p;PJgH}F)h!X_OU*4)VpEN#lb|G^AcZDFvIv^L#%Ar>H9Kh777>x3Lzwr24?>)mpG1rpwZ=pr54fBG5R4AQ5%!ky0VD?4hwr-eIw(YQ! z1ST@Fc4CbP{p_XZG^a5)Semyex@NLRw)i|BqS9O#pWPkeliTlApkLZnn)kXDSYItw z^s1~iz<8lWa#NE5Lw0GYStd5>qA)LC#9Ixl?Ui9B)-C6EySz?MgS+^FtHSNJZR7~=On&>$UE zdPpHKEXmIJ?TSEKRGIB-i<>?=Zti9Z*uK{Myo7!_nm{L?pj&pllp+l#p@NHI`Lqsv zP{n~uPKo)Y_W2)X(7eW*cs(Cir7V?^gDxdn;yo5*l3wT@(@D}9e}}v(=J~$zaOHh6 z%{<1=6EU4Het08cMd_2fo0^fkiCTLQX7XgVfqinRegE zTnbOH=xTAOVMmb~%K(6c^e~o`kmFE23Hl}d=F_Bg<0Nhsk`$yM^1|5*yGRhN#QQ51 zMPJ>if~lADQ*#k*A5}Xu4>K$W_y=IBm#5H7aT_fX#NU?sx_HI-=ZDP(2P}5`6)%qI ztUaif#(GDDFNP_m>GW6OCDYdh=%((L3$$OYH#DkD62EI}(e8f^btMGT%mMlMj(AC~ z;zVOw?9ln9VmZQnzh?AMI~9Fysc>W>v^7G+2B&j zB$K5*WImgeUY&H4BV9=|t9Eq1;BF_MnT8e2;DbN1+AlSw{YxW3C6}cOpZHvW$TwCmv9eBs1Sw{Dl{QWB)wFhUh~8FP2@f7cWo~l2lF`e&u;dn#YU9K1)e27 z^EUeuqV-iSV9M0iSFd|w;!RVuroy|{AxfEqEt>4+;=9iUqZrrWRPq7zJ{J*c;$;cp zFsi^PG7x6_F-F9C2EM2b!e4ZMATD)37O^q(q!d-%E_f78!~|y^HQg3@RA9jUXz;~h z*JPB2uA=R}Fd;1vDloFYtR<|Y1MQO&0?o!7#7s^F1cp4B$IDg1>>|I0?FDB=($6X= zg*S3|KVm{(K%|Swe9M5o>t$G8-E@=PpMWBKsIm!j*g(f^vsM+xYIe=ijty^fOE^Dj zIB~r;dtWr=Q#%JsB`9;vpwzirh>sON8CW^r;u$qea%E`E;V>kkp>K+zJGTqfZ4Z!Y{qpZU{?NN-eH#UUmrA?_rRu zg}L_umru@ZQQiD{tBH)lzA_zb#u1YsUpG)<_xdiz+9G$eYJJ4Y;`-!-7G;aRnwpqn zhUMJM=IRZ!9aPv%Em)YZYE4CODPYA zHW}i@z5GxxF*RrB+C=CiXJ!q@16cFpbck=wcrJSG#+pPL@q8`VfZkpnl1+k7+oX>y zRyuj1j4x(XRGdvhoz0$P#dS7t9D8*JMWlBd)nT~C5-HZw^J30AY?~6?o7>HU;&G0{ z*QZ#m0`eATk0?mYWJsE0LMuOQy_!fy zs?ET0(&E^**5L3t;^niJA*+`cu#)*BTb%t(3D39V@t#4D51iyF9SI9os`Ia!7*@PK z%Wk~g^X82~i4$L>{SLrQpGw6Gb7{B%bE2PfcS2x*TcexIuP=UF7OJJXvZ2v}^Vjd* zyxsn6I~nK2XSkF4FK@!l?BHN)GaHBR4poSPk+zFa2Y(IlI8$9e*hn3lZ>%lj&&r;g zL&6;&N0MDHiCkPiZu@#yP8P0C)jf08b}^NKt`b)LsVQV-zIiM#&fHUiN2l;r=?FaMJ0qpzrLQaqTqcT%;h(9jUd4)Y`n{5X^^DAN+w&(7 zs(3H3_TRahX>W116w_!}f)~rJNW)*ISXzuHBebwd7>>4rRNT72KfnM=O)TKMDw$wQ zMy1PK(MoiVFwP|*4X?CWk`CUb$DX1R048)UDz%UW*#}e1Zal?9=PjL2`j}#xF-^&s z7K?OO!rac?uYgfegTVN-2_DJZ1P{Gs^q0PY78CIco*oYesWvr)+F^*D-5l@8Fk$Op z* z_IN)fb})fI3Pj!%x%PhaLVx^9+DlNP0pwkBOtM8pg1TtlU{C$}KHIkO!1T@an@5?R zs?}c}H4rCUl|2B>pTkYy@7zO6H|a;p zMuR)*AY0PO`Q9oX5jBGJI$WG_cmy^ZD%J zl3m>PNcqdGf;s1j9X^~V{vt3CPYCdzc0~W}uz!z#*dJ9_`PIO$8$167ejgv;qT+Ac zJg0%bcFF$^w8SO)|J*Y_ZRd2e^rxi)+@|lT*6C^R>HgkN@MY3JcKc2nIK4>tX*@HlPdw9fwdF7CMA