From 76017f09494133c19ae9a093d6d291f390f54e19 Mon Sep 17 00:00:00 2001 From: Adrien Loison Date: Fri, 14 May 2021 22:14:34 +0200 Subject: [PATCH] Skipped cells are in wrong order This only happens when no sheet's dimension is specified. When filling empty cells with empty strings, we push these new cells with the correct cell index but they are added at the end of the cells array (normal PHP behavior). This means that we were going from `{[0] => 'A', [2] => 'C'}` to `{[0] => 'A', [2] => 'C', [1] => ''}`. We therefore need to sort the array to get the values in the correct order ( `{[0] => 'A', [1] => '', [2] => 'C'}`). --- .../Reader/Common/Manager/RowManager.php | 13 +++++++++++++ tests/Spout/Reader/XLSX/ReaderTest.php | 18 +++++++++++++++++- .../xlsx/sheet_with_empty_cells.xlsx | Bin 4724 -> 4627 bytes ...t_with_empty_cells_without_dimensions.xlsx | Bin 0 -> 4611 bytes 4 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 tests/resources/xlsx/sheet_with_empty_cells_without_dimensions.xlsx diff --git a/src/Spout/Reader/Common/Manager/RowManager.php b/src/Spout/Reader/Common/Manager/RowManager.php index 623c8d8..67c8fb1 100644 --- a/src/Spout/Reader/Common/Manager/RowManager.php +++ b/src/Spout/Reader/Common/Manager/RowManager.php @@ -56,12 +56,25 @@ class RowManager $rowCells = $row->getCells(); $maxCellIndex = $numCells; + // If the row has empty cells, calling "setCellAtIndex" will add the cell + // but in the wrong place (the new cell is added at the end of the array). + // Therefore, we need to sort the array using keys to have proper order. + // @see https://github.com/box/spout/issues/740 + $needsSorting = false; + for ($cellIndex = 0; $cellIndex < $maxCellIndex; $cellIndex++) { if (!isset($rowCells[$cellIndex])) { $row->setCellAtIndex($this->entityFactory->createCell(''), $cellIndex); + $needsSorting = true; } } + if ($needsSorting) { + $rowCells = $row->getCells(); + ksort($rowCells); + $row->setCells($rowCells); + } + return $row; } } diff --git a/tests/Spout/Reader/XLSX/ReaderTest.php b/tests/Spout/Reader/XLSX/ReaderTest.php index af7ca37..5b155b4 100644 --- a/tests/Spout/Reader/XLSX/ReaderTest.php +++ b/tests/Spout/Reader/XLSX/ReaderTest.php @@ -692,13 +692,29 @@ class ReaderTest extends TestCase $allRows = $this->getAllRowsForFile('sheet_with_empty_cells.xlsx'); $expectedRows = [ - ['A', 'B', 'C'], + ['A', '', 'C'], ['0', '', ''], ['1', '1', ''], ]; $this->assertEquals($expectedRows, $allRows, 'There should be 3 rows, with equal length'); } + /** + * https://github.com/box/spout/issues/184 + * @return void + */ + public function testReadShouldCreateOutputEmptyCellPreservedWhenNoDimensionsSpecified() + { + $allRows = $this->getAllRowsForFile('sheet_with_empty_cells_without_dimensions.xlsx'); + + $expectedRows = [ + ['A', '', 'C'], + ['0'], + ['1', '1'], + ]; + $this->assertEquals($expectedRows, $allRows); + } + /** * https://github.com/box/spout/issues/195 * @return void diff --git a/tests/resources/xlsx/sheet_with_empty_cells.xlsx b/tests/resources/xlsx/sheet_with_empty_cells.xlsx index d815a1c2b852441c702a1f9065e63134007659ea..8de9522c90b0bb85af382ab2c8fc4c5bd91bba63 100644 GIT binary patch delta 1448 zcmeyOGFe3;z?+#xgn@yBgJETKfk)b^rtJ?H7#OUXCQ5u}1lzhVcP7K))-FB|zpNtF)dCZ%Cum~`r$Sv6_wtE^A1H*H{&3f#snd-s% z7&sW3Hm(bLY|GHNfSG~e1_uL!1cMAiMUH-Xeo=ODMrvwFu|AkG)T_wN2@T<7V4i$) zUIGZ0R&X;gvU~?x2l58P-Z0<%+ZFUH^#hC5j|uxc`+{s=2^|n%5vUTJa_q|V#1)%`v{tEaVrw*R z4G}f92>Bgwoh##=S*T6V;ukt=G=v-7cZB?Bbrxei{lG2Fm-SGE0PCE3sU_>Z!(J=+ z#5v}0DB zUV2$;^au0i#d5Wim4$cO^#F{nJ9t zpYBsQt&QgWV4w1ax$CX>k~;3zm-ULgcXJz)i%uN>eXaN5jFjIYZ<i7HMQ0?p@mO za>=Vl>^iA}9IMUUJtA+5ct5yvO@6|eb$?raq`znSq?5Y!tcLs7Wq~GA_jkTs9=Q6m zZtbMr%d6*ZZO>4Dx|lce&WW<~p^2sISK1cnb~gm{$Q{_cb6)T9I_-=1W0tQ=cUIQ@ zQ2%51>zk)!HEo#J3a$9zu#VUAam{7b8Ek;o-US%io5uJ z+rEmv`Dfy^s+;Gn3D}l0ZMIPI<&!pj_guv!TZIziqWz5-236bFl2_r>|W* z;supn&#U^=z4Ewn(xaAN+)10`eR95a-T1O}*>(Hqhb!9`FAmgJJF}~7pSza{kI9>Z z3E!G`&T+f&ch~DUucPyp8cfl@=;PSiXR?o@Ep@%RQ~tvFD{mS5oSDn&I{9<==gQaK zhXwN2RR=!%Soy!Xe%HGtj*9gr!B+RB8Mr#jta=XL+c#V4>_WD!spl(J|8scg<`!bG zw0G<5-OFw@?zcPlYX7$I^|If@?G(hy*2d(O6n+e7idf8aBmQUr!z(_2x?FSycrz4G7lM2i=hN}uoScX$~cb@l!9 zci(U4_nN$y{}6cP37`C)>d3&4w{72iObo8Ow08EfcjrTe_zc7@PX1Zn5hna-pE=*Q zqh5y!=UTk^ukLyO4>zpT;fEAE+{qk{vw*qx(&U%C+fmAkc(Xkb2Efc6&a(L!pEn~r zBa;XNFrQD>5U^lm02WP(vYbHKl>Fp?qWpql{ltO-Pze#>4O2H?K#@^$@<9O^pz3=- z_OwZYsz9ciprXtI76w=mla!yIjjAtMP|@ZX8(2YcNo7uIF-%&H8C1+*E3hPh;tUKx z*Dx$;Y@d8UP+BoGgcVXyp%qdH=RFZrWUAzy{Ek;_vaApX$YDZ?jINVCgp|0#fu-a! Hevpv>(~wN$ delta 1590 zcmbQN@N|OqBSn#?Fzps%iTJpqMoi0|PFVlNA|%0hKb% zntJ^aP-!|NZl#+InbH}7YPSX4WcLHATZYfrb>w_sn zy^7qNyz z2lTz?`YmsNp}^RE^j-b!?FX7K_21W)lX1vYJE}Ct?~wt+MDJ5a>ICa5qy83&vfUDJ zM{`g#c}=B{v#7y4;Jm1Tp8SGVAsLq$8+eq>gv)NOZ5)B)w8lbX1kK^ zDOl#XgDqzDNx5wa`>y2OwK>Tvwd<*d_u{hRX+dq0I@*m*NrLx$)=xKn{ZQs#O2L)I zg#{^1ryjd`GVd%5`8A;D`A8!w*7FWz81ud*l1K!lb_7w*T5`&#ONvNgog2oyC9A z?^JsBro!agw&AT$_o{t6uvOzw!-B62a%bP>*nE#>{(H9cRhgN|B{sSK-SgJonRv+f zf>C|ro_eO!T1?(*;fdRqe?A?wCU$rJX(sip+ibSqncfp55fwM_{kDB6ea>n6Q};UR zPt%AtTD;8Vc8QVyqwgQDeo2}ZbH+Ah{$bU9%55ntCmna#$xyktca6;)#eDYfB8}hQ zX=U|Zl5efHU?>kb$93>mj@9j~w>AZv_Xf1<_$_W`t-n!qXI{0>{VJXFwv+BDAKLZn z(Iokp7$YV%zpiTLoNvvIXU{dvEvWjPFDF-JVr8>6>Cl?&S|{n_GOZd!l1IXnB!E#ruy zNaNe@RxUj$ytC*F>l9JeZ2=PE^$k~MUZ3)>OoFd!nwQsK>8X2_L;ohe+;_h=d&$oa zE$5f<$$jvwnIbA^Z+}Q`zTH#tx%W*9?aebyS~d#W&iF5Wu}Emg$78Lz$xAQp@SF4P zf7GV^i~-*49Kw>zb(R84hMye3Y>lHZn>>McJFp<(PUdi&1fqVrf@yWIVT#S~J zJp^Qc{JG5HlWTzdW0TtjRDt}p0*Z_eCZ89O0j4iHe;6mM8UV5kM#-w}N640(@)fR1ww9Q3DwoK@ZY;Pwk|iiCysy3J3$6 zmlP1iYX*PolU7Nep}ze+=DB)|8nV<}FgEU(As{cCe1C(f*%jW$^}$4oo5G>Hbfn#F zmp#cHuZl@*!cre=YAv^5Tfp29iI_nIJiKdVLvV&z-HR&On{}E!dTbK8|L|#L$%a~_ z?zp9~e4x?_Po2c(WfG(JC%;MF2lxJqWQFUxui?@73 zBq~#pX5}H~)JwJ=tQLRTl%%uP<23qqkk?5O0Q(ma`01(R3pfJq-~j-XxD|nPfT55c zUZQp$9wPVdx@8V1xVDK=g65lBuu(5-jSMp=5#cFA`~&3fYqyQop7WWqX{^j2AC(GA zKD*Akwf%K=d(*yXfednGOAciNF?e8T&E&hn$W(M*KN3G;5{a9!3w2!8qDx{Jrj zr~#$7T<@fUxbf+%gG~wr-iz?&#hci(E`}U>N|IzXr3=gCOF0OK16!R39DfjWRn*Ha z-q+t8^=;XmwNc(9+*0T+k)~H|Tl1c8q`aEPy}bBtHc;)}!Jw1jtJ0G7(b;;xD>ueA z2X4?xld8A};D@I`dQ9T0pP|jX{Chn8gA#w>|C^6m*d&oIj*CKEVNm^(4+kX5@tlyy zNlJQc>mF1*eF-gj$oKW#sr3&9cbx_& zbFQ?QEG5&aJdU-Jh!Xz|(YlxHS&C$yrt8_|c4NXP1QkFd-$?CA8}`?n@vdiEOHY!z z%tU!xVG$vjtBO{d5h`uyw3AcwrOW>&U+?RI{}7IJ9vu0Mxb>c!=uct#B2ljPNTloe z0sItcq7JSs$s)E+=+yS#X%aca$~4C+Rh<05>(E~qXup;;R&y3C!el|>_<10CVEo$~ zjqmG{kJqExRmec4xrE`Z?|i_`AtzrpKVC8E%blHf;)$h=nTs(2wuAj!WzA>XO#})m&k4M0wmHT#tH4xJupb zj4I4ee3#A%L^Tzxw%k!QbB@^?2vhx#D~-IJML%9-?oT9U;zU>N!5~Iqgdu%V3pqMX zBQNZ&?lf*KFrloLF!gCZu9R`9?s+}yx7%C=DWlA2*qUkzO`m$fRGMj^%pj=BM z+st<}TGK&%-n_N?{h81S6NYMMnMY}n@&Qgnbk&nW2y6aWD7^iJ$NDFLQAJHL$C;J7 z`<;_Deredgk>MFR-uZ${FCC|Kvm8<2~sj7%J)Vz-In+c zQbwCuBwFC@J9PinOMVS~;}e`#R&br_($6aKLi@QndY$WJGtEkGN{lvQYhO@kE5!$+2CQV&t*Y_3bz*7mF)}%3vb3uVM291 zBc{SngcRh;vBkHHx)C>8_p8@dXkue|#Ca?=hr?W`DD$7qV}IY# zrcBGv2~8s~HXzm$F95z08}YN%($RFWOqy>MBYPjherrJ{$qPa^mZREN0AwnE82C*A z=qZowTof@TF?nNUWiyj?_Ew1*c!@nuoCp4#YEPN@+2vhTnVZU&MWSUKxj zHtj4_svWiq54B}#RxrV7JNztCIB@?>Z8tAnq6HPN^_Nl$2-=k>6viE?pfQ?K46NSN z>!9Tzm|S@IsFjK6i6}3$|+WIIb!()$pOuzyWtWFM*N>Z||a^s&>uOAV?M34w=f6g`brr#eavr z7K#ur=Z&~iIB@I6hmn+;_!qYvT?sr><%iJDRkr;3KAd6DT9XnFZ;pBbS6CG% z_840_XXUmI57al<=5^U53qA+3*Eq5)#>Gq*Efd-_BHd?8rrVx-_8=s*mO<8^u&$X6 zW00p>;HrGGIRCV^OORFgk4EU@fZ}aXal@F=Rby?rFNtay$^LJ2{a-J(6wSag-^SGC z470AT1`VutzFh#7wKy%n|14kt7+_V6G3c`P;7-7`xVK;cfC0dK-tf=coR^cMBic*! zj|FmG%f?2;00OF+EZg(1_dQYo9^pQ&JDwXlK(mPsa!Ll*3*nS(>XRXzhA{>H9~IgXMF8R)AC6)%l;j)UbsrX9%S(N-!u(LjWtYck8^$B;+F~{BB)rqqd_nvlg zvK4UCTG7Tt1K6%iL^Vk5jG+frn zO?o&vY4eD{Za1Dkes3oTnUkaq#vs!OSnvBPSRCYqHM^(YoKXxlq>C;{?kS!cF1J9K zHg30Ml7xe%F61BEzm@a<3I*_3J|jzkD@c1*{=m*)*`dZ!6S=6fyhT%&^|Z_do;aoFsH-z(&g~p8sWQLZkr)k#iw>{L zL7+VSAg)2q3=4hVMc)fGuil4^XIAljiodP36+m$F#ei<4=kyfXFi&3=LDvZbBiY6LhclO?$smN%|tp(AUq z?_j&QOBBR%>@eWR>vZbDGdZ3#b&AJvYd8_UvTCMn-0-wprZIJ%176gw*NC?AlCQ^f zU=&oi;ZnYvS#r-p69h%s?3(y0!-xEjLkuokOYE)K$*exQ1);ajdFN_Ay7lCAe*^ir zYU_(1Fk*Tk%(qBD;C^G6!VNdMTZi7mBWDfW3wRwIAn`MbL1*RT{f@R;Q!{xCP2|ZL z>m&Ht1tUZ109H!Jsr1~|ovRh6a4#o2lq13rjdFIs^T(vJic=!Nicv!Mvp9Qa3k9My zl5&B1IYi*i2~fIb-z_K-303ttm|ba=9_1omk9(%VNm_>Ua3lHGV0S*EZV3D4n9A1# zYdoaS>V!3f+tEuEwbT12dR9nDE2#BD855V6(owtO4qqhWuLSido*MwS44?JIg&KR>($){(IcoTx&8F!?hhaApk&wdlw58 zq&wQt9c^Re=i%sOb>66&Qu>XVbuq|LclB9FkTor$te3pWJJznn2)+|iG##+S;n*I|dUdOh0H=@No8d|UrJ zjPxL8AW>*Ypz26e$$m^Ze88-nv!7S`!*TA?bpwMqg^}NH!l&6k&2cY&#}8uf>VU3a zCtiUImW3$H20Wn^s~mdJMPPxiM*99$jk*lWY0!B7jU*X#qm!aMX0FMGcl~~Q?|jfF zGk(LHun6-9oo(?otEt{ZKWOV%1`5{l)~KY}fZXrJo4-;jo<>qT(S`cObygfKD18pt zT23=U9rNAJhH2yB)1DW`|H;JV+4zA?>9FBkGpo`#(X7Crd1N2jOe_O+IC@aCa z9RF(!GyH+Xu?ENwi6s8}zW*_lfj@`xg@JtW!7jFdUk~PtGp+yC5H9X;QFy<0kRtz2 X8N#%IxOWBsh;bJjcY;sOGZf%|xIE8# literal 0 HcmV?d00001