From 2a0c4cdde470a9641c0c56f8c7f7e689992e81e8 Mon Sep 17 00:00:00 2001 From: Javen O'Neal Date: Wed, 8 Feb 2017 07:20:31 +0000 Subject: [PATCH] bug 59983: correctly update shared formulas when shifting rows. Thanks to Luca Martini for the initial failing unit test with test file and Chiara Marcheschi for the patch git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1782111 13f79535-47bb-0310-9956-ffa450edef68 --- .../usermodel/helpers/XSSFRowShifter.java | 15 ++++-- .../usermodel/TestXSSFSheetShiftRows.java | 50 +++++++++++++----- .../TestShiftRowSharedFormula.xlsx | Bin 8431 -> 8527 bytes 3 files changed, 48 insertions(+), 17 deletions(-) diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java index e55b014a7f..46f0b892cf 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowShifter.java @@ -142,22 +142,27 @@ public final class XSSFRowShifter extends RowShifter { int si = (int)f.getSi(); CTCellFormula sf = sheet.getSharedFormula(si); sf.setStringValue(shiftedFormula); + updateRefInCTCellFormula(row, shifter, sf); } } } //Range of cells which the formula applies to. - if (f.isSetRef()) { - String ref = f.getRef(); - String shiftedRef = shiftFormula(row, ref, shifter); - if (shiftedRef != null) f.setRef(shiftedRef); - } + updateRefInCTCellFormula(row, shifter, f); } } } + private void updateRefInCTCellFormula(Row row, FormulaShifter shifter, CTCellFormula f) { + if (f.isSetRef()) { //Range of cells which the formula applies to. + String ref = f.getRef(); + String shiftedRef = shiftFormula(row, ref, shifter); + if (shiftedRef != null) f.setRef(shiftedRef); + } + } + /** * Shift a formula using the supplied FormulaShifter * diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java index 42773aa6fb..78e9d1bb25 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheetShiftRows.java @@ -427,8 +427,7 @@ public final class TestXSSFSheetShiftRows extends BaseTestSheetShiftRows { return cell.getCellFormula(); } - // This test is written as expected-to-fail and should be rewritten - // as expected-to-pass when the bug is fixed. + // bug 59983: Wrong update of shared formulas after shiftRow @Test public void testSharedFormulas() throws Exception { XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("TestShiftRowSharedFormula.xlsx"); @@ -437,17 +436,44 @@ public final class TestXSSFSheetShiftRows extends BaseTestSheetShiftRows { assertEquals("SUM(D2:D4)", getCellFormula(sheet, "D5")); assertEquals("SUM(E2:E4)", getCellFormula(sheet, "E5")); + assertEquals("SUM(C3:C5)", getCellFormula(sheet, "C6")); + assertEquals("SUM(D3:D5)", getCellFormula(sheet, "D6")); + assertEquals("SUM(E3:E5)", getCellFormula(sheet, "E6")); + sheet.shiftRows(3, sheet.getLastRowNum(), 1); - // FIXME: remove try, catch, and testPassesNow, skipTest when test passes - try { - assertEquals("SUM(C2:C5)", getCellFormula(sheet, "C6")); - assertEquals("SUM(D2:D5)", getCellFormula(sheet, "D6")); - assertEquals("SUM(E2:E5)", getCellFormula(sheet, "E6")); - testPassesNow(59983); - } catch (AssertionError e) { - skipTest(e); - } - + + assertEquals("SUM(C2:C5)", getCellFormula(sheet, "C6")); + assertEquals("SUM(D2:D5)", getCellFormula(sheet, "D6")); + assertEquals("SUM(E2:E5)", getCellFormula(sheet, "E6")); + + assertEquals("SUM(C3:C6)", getCellFormula(sheet, "C7")); + assertEquals("SUM(D3:D6)", getCellFormula(sheet, "D7")); + assertEquals("SUM(E3:E6)", getCellFormula(sheet, "E7")); + wb.close(); + } + + // bug 59983: Wrong update of shared formulas after shiftRow + @Test + public void testShiftSharedFormulas() throws Exception { + XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("TestShiftRowSharedFormula.xlsx"); + XSSFSheet sheet = wb.getSheetAt(0); + assertEquals("SUM(C2:C4)", getCellFormula(sheet, "C5")); + assertEquals("SUM(D2:D4)", getCellFormula(sheet, "D5")); + assertEquals("SUM(E2:E4)", getCellFormula(sheet, "E5")); + + assertEquals("SUM(C3:C5)", getCellFormula(sheet, "C6")); + assertEquals("SUM(D3:D5)", getCellFormula(sheet, "D6")); + assertEquals("SUM(E3:E5)", getCellFormula(sheet, "E6")); + + sheet.shiftRows(sheet.getFirstRowNum(), 4, -1); + + assertEquals("SUM(C1:C3)", getCellFormula(sheet, "C4")); + assertEquals("SUM(D1:D3)", getCellFormula(sheet, "D4")); + assertEquals("SUM(E1:E3)", getCellFormula(sheet, "E4")); + + assertEquals("SUM(C2:C4)", getCellFormula(sheet, "C6")); + assertEquals("SUM(D2:D4)", getCellFormula(sheet, "D6")); + assertEquals("SUM(E2:E4)", getCellFormula(sheet, "E6")); wb.close(); } diff --git a/test-data/spreadsheet/TestShiftRowSharedFormula.xlsx b/test-data/spreadsheet/TestShiftRowSharedFormula.xlsx index d8ac0c6e2e650e7cbe896cb589a16f7d11507c59..8fc3c15a4a263bdd3cb4b965f268bf0bdd5f25b0 100644 GIT binary patch delta 1964 zcmZ8iX;c!37RCj0DVIis1Oc}vP9kQS3u^AWnA)PHm5Mt^)2LwDu;kKWnqXGq85ht@ zbIZVOcslM&shN%?l}lFkiZn#6RD907bLyOTf8Be}_vgFkyWh9uH0s;{Rie({7Wmo8 z0suww0Du|*0H6>sWYYPB2ofp5ltLujKk7#kn(Y>-J)^28kvBEk{jNWAV@C(m%TUv% zf`zVu^Cqr^c3!6gH8++xqSWC+pKMV?;NC6Qe|yCLmRe=_b?7?iWRyj*e8IWiB;AQa zoRSg;%0ch@a>r;BS7RiZ)KeGkbWp8k5}Jgu0)53&g8Tk=MHB1j8p@uOa%kN$ccM}u z+ox%j7-_A^pMNnqp2wJdRPThcMRMPwA#EnW*H%S5%M*!!1EGv=3DHm+TR!;_ea`ZB zn2_Q-wa1x|q4}v|R|Plvz36zH9P~;Qq6_cpzAoRU^(eN+$I6L{qI!zCugsRa&!KEe zZaIr59kKMv)=O>eZh?A#wngc}Fy~ape~0aDK;9bBWtrb&h*e2<=_>D-TotO&2MLQY5*B2w(q| zMwS#-o!;(GJmd^(8F`hmHz*>Ip4*DE}}AH+5%xhL@w z%bZI!E3z}S%g;VD74yzuEGyYC0l#a%ShgrM`b-Bm?H)XN&skagqU7pCpG2vmXll&dKHom+ zO9hi!9i>no?~`0lyZI?VIELB{ei&7*|GRDgV(!K&nhYN8W!CjAC3;1hZ)avtpjs5Rvdtd4>+A( zyp;Qq7O?|*aJ9M_j}BXVVHY~+)RXt6^dS-WQ@vOPf?FowNzI8@a!K3+4|EbOrJ*up zqk%xHtWN!*$XS2>T*m2?yojS3tg1#;{@H+ zI>&l;l$}fdim%G?Np0mM+3dG(M1CWFeF-X~{6l92@?an{WuFyrI*nwWqcd|U!c<9B zr-w+Sf>!2Y$MTJ)&sHCZxbgOEjVjVQOIb5^$;=NW{d}Qe zXv=2ISpP0litoa`Yl-B~3anf5aH@Tkam?uHUd6A*zn!T+-F7VUp0Lik)EsR0TI+T_ zvDNmRR&7YE*Dx$EgWq%2rbG22$?$T2Q0ucJKG9e*GE26*lN3ZFw@a*Omz~@0&%7!B z!w>AwHvHKrqw>I=-hxEbpy!wIyhGiS>1_UX*@&0a$e{)Vo!pS$xwHEJ@*^^w5a|*d9+&ucfQ05cDR-HvyR9?GicQV;7;fA+ zFzMb|Yx4ABPKA=bNr!M;p| zIQudh#yyt3Rd;;68t3C;0!1m08T+}N7~zaB;!QR)3Rt()aFv@``TfSFuyS|b5pht+ z@fT4t7Q6^2$1^eqQ{~JyI@0@^ftXSuLuv<}8q!Kq37$-UIkWi>6IKmUwTTQ(kyH8} ziL5m^P9}xrhH#Q`Wo08AWfWNh2qf&gp8){?$PVQn)dFHY0%^10NOkCU?d=$!#t+0! g7tHcP8bJU5Za*NeS(lOaGVQEBq`pEL{O|pL0rl;Py8r+H delta 1871 zcmZ9Nc`zG@7sunK&Yzo*M8&)Au zm6i=#M=2$$!baJub)}AW+oiQSznOOC_h#PAoA=%y@4flX=RG^aN45bah#YGLwxu8d zKobT4WB~vGAqJKd7atuK7Z+_pz{NECkHvCL<$6`t9OQb(t_QknAbsT=sG|B*NJ2gh zX6gI``J<0pBkSBg?_qx9EE8rhPwR%F*%hTao!lD5HA#!2=)x7ee8`|8M+f~-!-2`W zW|7mZ-3UEnRJ5YdN&LVf9O&9ccJiY7Tmh9axzTTK&-`u7EQKak1M5TXK6jB9^XuHh zg{vCi#!DE#<)tr;7L$$aAhoNfRC^k2E_--kMhbukDTP7gN7m|1%_(S#bUE}2&kDrp z5V&EivQTl|CkN`qPjUx#C?O71b+ygODwFwsHO!U;;mSi|)v)BF(50+bhzG-lCUbib z+ml{<^x7qlPkEX;=m-BHe6D|QZN}3+ICRg zX?<#sCnP~n!-8tVnj9r%I?NoQ`>t;~GY2&Hhv8idrjb+>>tqJ3%HZX^*mBg2I(@~t z=UNAXNYuXI-cK3#1s6{JD(zN%~bQDgjyz@}_8Uya9+5zbr2J_n~}E?UFOr}ECe%OtLwIEq_0 z1NpYzX66Q2{R_pRd8VITvMWRI*8ACR1>_V_AKM<;StYfcO5O4q23sC3e zW?1OMlr3ybSbTUjR-Oi(h!J|tezcMgvSbKEI zg}5FglQJ3OfNJF{abDRk-mX=})P*&v^FDol+cfnptIXoS1+R1eyPUkl8Kvu=S5H^? z{p7y#-zCX^Sji`-TLdU5wHM zpQrX~CmgD+&-v;JHTvB>_Ev{_6x^{Qa6JZ*us)u)KJ)R3|K|FT<|8)S#sv3rAbB*5 z8byacwoAp3r^0sa>nWN~D)&n5BXEZG5IvKgm%rw<+@;(M<>yn*CB~~I?mit_9E=PT zL`mbr6R-&b>wyw^o`uyM;~;5@7q}ktfFKHQxw-j|=S16`LQ5B`tc#RiK>f8MaNXKl z&o!(*IV^uL^EhIhQ&p0{&Q{eR1pokSXPhcTfS8j%5Y2|D4)HTS$qpn}vuYIGeCJ%e zjiqMW+soEo1SsEZxHx~5s*CN6Sqxor=Iy;X0Ikw$TJpW|L7aMorMA9+cGSJ6EeKfI zwjD@!%%}X7F=6J!>28Ou-FP4JXCzlL_b?>xMz+-3;L9L43LFDQRo(# zbiHU0#@z6Uc9n7aGQ2sQhY{KRyg!y~&vcEoc99h;9Sms(uT=MW`}t4@_wvSFJM$7I3;&il7W|0B<@&C!tNYne(c!g{)hYU z(3o(?$WUzT7v6j3B2WBgD(C!ndXji&7gof%7OQgJyHZU2uQ}m-w-MnfE^vTaCVP}Z zIAjq;VW5&ZXWvQxQ{qqsS643ji96q5M*wHN_llt3(_4vZAElVXHODKS2Q4Fv!UeFy~j6+R*W zfa>wC->i)h4lx3L71tL)eJdVEDvWy&DDeMp^{pk3u?Ddhz%X>Rv_*lSFZq7~_?J;Y -- 2.39.5