From f73db4b95c5f92797aba34764956b68fa9b44ae2 Mon Sep 17 00:00:00 2001 From: Adrien Loison Date: Mon, 11 Jul 2016 18:20:05 +0200 Subject: [PATCH] Shared strings table without uniqueCount and count should work Use file based strategy in this case --- .../CachingStrategyFactory.php | 9 +++++++-- .../Reader/XLSX/Helper/SharedStringsHelper.php | 6 +++--- .../CachingStrategyFactoryTest.php | 3 ++- tests/Spout/Reader/XLSX/ReaderTest.php | 14 ++++++++++++++ ...d_strings_missing_unique_count_and_count.xlsx | Bin 0 -> 3609 bytes 5 files changed, 26 insertions(+), 6 deletions(-) create mode 100644 tests/resources/xlsx/one_sheet_with_shared_strings_missing_unique_count_and_count.xlsx diff --git a/src/Spout/Reader/XLSX/Helper/SharedStringsCaching/CachingStrategyFactory.php b/src/Spout/Reader/XLSX/Helper/SharedStringsCaching/CachingStrategyFactory.php index 8fffdb0..36e0bfe 100644 --- a/src/Spout/Reader/XLSX/Helper/SharedStringsCaching/CachingStrategyFactory.php +++ b/src/Spout/Reader/XLSX/Helper/SharedStringsCaching/CachingStrategyFactory.php @@ -78,7 +78,7 @@ class CachingStrategyFactory * Returns the best caching strategy, given the number of unique shared strings * and the amount of memory available. * - * @param int $sharedStringsUniqueCount Number of unique shared strings + * @param int|null $sharedStringsUniqueCount Number of unique shared strings (NULL if unknown) * @param string|void $tempFolder Temporary folder where the temporary files to store shared strings will be stored * @return CachingStrategyInterface The best caching strategy */ @@ -95,11 +95,16 @@ class CachingStrategyFactory * Returns whether it is safe to use in-memory caching, given the number of unique shared strings * and the amount of memory available. * - * @param int $sharedStringsUniqueCount Number of unique shared strings + * @param int|null $sharedStringsUniqueCount Number of unique shared strings (NULL if unknown) * @return bool */ protected function isInMemoryStrategyUsageSafe($sharedStringsUniqueCount) { + // if the number of shared strings in unknown, do not use "in memory" strategy + if ($sharedStringsUniqueCount === null) { + return false; + } + $memoryAvailable = $this->getMemoryLimitInKB(); if ($memoryAvailable === -1) { diff --git a/src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php b/src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php index 350024a..0f41e90 100644 --- a/src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php +++ b/src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php @@ -147,7 +147,7 @@ class SharedStringsHelper * Returns the shared strings unique count, as specified in tag. * * @param \Box\Spout\Reader\Wrapper\XMLReader $xmlReader XMLReader instance - * @return int Number of unique shared strings in the sharedStrings.xml file + * @return int|null Number of unique shared strings in the sharedStrings.xml file * @throws \Box\Spout\Common\Exception\IOException If sharedStrings.xml is invalid and can't be read */ protected function getSharedStringsUniqueCount($xmlReader) @@ -167,13 +167,13 @@ class SharedStringsHelper $uniqueCount = $xmlReader->getAttribute('count'); } - return intval($uniqueCount); + return ($uniqueCount !== null) ? intval($uniqueCount) : null; } /** * Returns the best shared strings caching strategy. * - * @param int $sharedStringsUniqueCount + * @param int|null $sharedStringsUniqueCount Number of unique shared strings (NULL if unknown) * @return CachingStrategyInterface */ protected function getBestSharedStringsCachingStrategy($sharedStringsUniqueCount) diff --git a/tests/Spout/Reader/XLSX/Helper/SharedStringsCaching/CachingStrategyFactoryTest.php b/tests/Spout/Reader/XLSX/Helper/SharedStringsCaching/CachingStrategyFactoryTest.php index ea77b4f..e61760a 100644 --- a/tests/Spout/Reader/XLSX/Helper/SharedStringsCaching/CachingStrategyFactoryTest.php +++ b/tests/Spout/Reader/XLSX/Helper/SharedStringsCaching/CachingStrategyFactoryTest.php @@ -15,6 +15,7 @@ class CachingStrategyFactoryTest extends \PHPUnit_Framework_TestCase public function dataProviderForTestGetBestCachingStrategy() { return [ + [null, -1, 'FileBasedStrategy'], [CachingStrategyFactory::MAX_NUM_STRINGS_PER_TEMP_FILE, -1, 'FileBasedStrategy'], [CachingStrategyFactory::MAX_NUM_STRINGS_PER_TEMP_FILE + 10, -1, 'FileBasedStrategy'], [CachingStrategyFactory::MAX_NUM_STRINGS_PER_TEMP_FILE - 10, -1, 'InMemoryStrategy'], @@ -27,7 +28,7 @@ class CachingStrategyFactoryTest extends \PHPUnit_Framework_TestCase /** * @dataProvider dataProviderForTestGetBestCachingStrategy * - * @param int $sharedStringsUniqueCount + * @param int|null $sharedStringsUniqueCount * @param int $memoryLimitInKB * @param string $expectedStrategyClassName * @return void diff --git a/tests/Spout/Reader/XLSX/ReaderTest.php b/tests/Spout/Reader/XLSX/ReaderTest.php index 3fa6ec1..02c91ed 100644 --- a/tests/Spout/Reader/XLSX/ReaderTest.php +++ b/tests/Spout/Reader/XLSX/ReaderTest.php @@ -126,6 +126,20 @@ class ReaderTest extends \PHPUnit_Framework_TestCase $this->assertEquals($expectedRows, $allRows); } + /** + * @return void + */ + public function testReadShouldSupportSheetWithSharedStringsMissingUniqueCountAndCountAttributes() + { + $allRows = $this->getAllRowsForFile('one_sheet_with_shared_strings_missing_unique_count_and_count.xlsx'); + + $expectedRows = [ + ['s1--A1', 's1--B1'], + ['s1--A2', 's1--B2'], + ]; + $this->assertEquals($expectedRows, $allRows); + } + /** * @return void */ diff --git a/tests/resources/xlsx/one_sheet_with_shared_strings_missing_unique_count_and_count.xlsx b/tests/resources/xlsx/one_sheet_with_shared_strings_missing_unique_count_and_count.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..6ef7ffde055059eeb48c1cb3d102fa471964f5a0 GIT binary patch literal 3609 zcmai12{@E%8y;lFIwV`NBw5Rr!HAPJ#4wH}QIuscVr+x4hGRLh%aIc)ODakXp>U*; zYzbMjS9Wsj8RY+l7}5X#?{$6GHP?L4{XXw=zt3|&&u5@Zb%+%N0?~lg| z9|TWG@0ouZAUT<`@#3ttejTcUUu4vD@!H-A{ed+sFIdDvJ001~j4)}dwM{D(Nz?X^ z3WFtZDs_XnUavU+g|fi8KnvF|SLbLEj4CI4BtO_nLBCFYbd<8J^5Pa1IWF?y_FTFD zGE<3yfA|rd#p|u2Q(!ZLzUL|ry~cGT=8lD3;}w|ahrg)p`dZ~&h6+9xZ#&v5R4|fF zaSM`vD4M;N(;-gEa@h5ZAyF73x~AMX&13L`WDv+WaY`HDb$5Y*Usv5xP9BmwHrp5B zt^DjOuFyrHe$-}G?5WH6! zbi&mzRw9CT(Rbge@OrQvp?)!O!tXSG#8Mb0u$&z?MbucZ2h*6;e-y}j>Kt6UEHRJc z`%L4wk~S`XzHllnqtSs^DQsSE@OEf?f}fap)2%??yu3qnz89uXoY^pGkkIxKb11wo zM`9T3jR~4ZP#3Trug~<>Me{v;ZHrE;a`4 zF0T7&WBt8zuo8HE7RX`dy=GRfuB7C>8()2sQkq#~mj1hXorTO-RgE(OAvjCdPCsp5 znA8OH!YSfPN!g>ZnaiHlS-I21JDQ%sh5}Z3V?~;$RFM3i3OlMcFfZG4@q{5wHHRg} zTubj;l50jH>ccc9w3OHLkWD@)#FDUfeshhc8(6Hqu+AL@V`wELuPadCS21_Of=^L_ zX+Eq|P71qu4sWVat!n+_JMLnJOEj(TN$-vcK(xqqR=l}aUhfkAyl~#I#}?7!*-6&Ff0XRfqZF0G-mg9sht`~v%rk+Tm*H$M>8%uL z%N)axi{c^A*c~~;okWBrnvrvBA3irOb{PNdW=d($XTXB=@eQJdC^#p@c3wBJyL~eU2_vZ&Kz~QS6X-Fs-Nn z5^+xdC+`4*UzMvEvzKKjfY$-am1}#SZ8L8d_v_X!F4svH@}qj+ITEB8f@Z@i|7Zb<^#7LrAraFHMa!Z243S+)(EY9Rf6Xi8~)jg#Xr;&V`OP~C>+l5(Hq&XkmWMKt! zq@Kfq2f8HV-WQ}OBPLC`ost!&*J^ZlspW<|^kts*_;8DJNL$70A436c{o_Pqil_^?r4m?2Z@Y;=xVyxQa~k!Y%(toJ;? zOZn$&sVK3_*7Q+zUafcDQfG~wb=D43GK#g-X(}2O&$@?GO)8trN9&&@su;u?Ckzio z!!db|4l(TZSoHU9&j`l;=C8NeY}~GrBO&#}OFFq@3P6F40QbuHBND8S6KX32PeVNy z!f}R5s=r>1-KOeAj(hsn#GVxn? zg#6_;taQyfiT7QwRdr<~4cZ((FbXy*p5>6Yruvg3;$w;y^oT6mTp|r+a-_9Q#f(3f zK#rCE`DZH-X48kbF}1tj$8NL+r!{aH!2DcW^c3GGE)?}OA68J*?J}2xK6p_YFgC=X zHO0ocki~mRSuEV|vN`byeR^m9y#)otGd@DbAWf_zL-tC}DT~2H;>ruoCE^lm*T$8N z-5T6(bxd|igtq{WeqeI>PbXuI`s~;%Dw#KVf=gkNN{S{z4Aa? zX1Ji%`+yR;UOp>!BPFp3qt{_@j=s`$!-fK|Eu<&tQG|Q0J-#}D*CYzuA?VoMPuvdFwKPDZr)9otTKQ55bc|haa zb-FV=kl_ajA~j0#`WN_A*lL?Qr#%SxImpA*TYz7ay1$W<{lNFa_6o#4j(^>P$opsl z_xr1cM2>iyu>I0c-hmmiy~A!J*bm*WXk;jsW(&Ia_+;=w;z-g!UJd|{z{yGAU7^?^ hod)zPsUTyM4q(Z`VW3OBmGe*#6r>Ma?sGa4?myU>J@^0s literal 0 HcmV?d00001