From 86a4c3790a9837b9b15d17d8c0c48075800e6107 Mon Sep 17 00:00:00 2001 From: Adrien Loison Date: Tue, 21 Jul 2015 23:00:40 -0700 Subject: [PATCH] Adding more tests --- .travis.yml | 2 +- phpunit.xml | 1 - src/Spout/Common/Helper/FileSystemHelper.php | 2 +- .../XLSX/Helper/SharedStringsHelper.php | 3 +- src/Spout/Reader/XLSX/RowIterator.php | 13 ++++ tests/Spout/Reader/CSV/ReaderTest.php | 63 ++++++++++++++++++ tests/Spout/Reader/XLSX/ReaderTest.php | 62 ++++++++++++++++- ..._sheet_xml_not_matching_content_types.xlsx | Bin 0 -> 3757 bytes 8 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 tests/resources/xlsx/file_with_sheet_xml_not_matching_content_types.xlsx diff --git a/.travis.yml b/.travis.yml index d4306c9..9e90442 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,7 +12,7 @@ install: script: - mkdir -p build/logs - - php vendor/bin/phpunit --coverage-clover build/logs/clover.xml + - php vendor/bin/phpunit --coverage-clover build/logs/clover.xml --coverage-text after_script: - if [[ $TRAVIS_PHP_VERSION != 'hhvm' && $TRAVIS_PHP_VERSION != '7.0' ]]; then php vendor/bin/ocular code-coverage:upload --format=php-clover build/logs/clover.xml; fi diff --git a/phpunit.xml b/phpunit.xml index fc6d657..06ddf63 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -5,7 +5,6 @@ colors="true" convertErrorsToExceptions="false" convertWarningsToExceptions="false" - strict="false" verbose="false"> diff --git a/src/Spout/Common/Helper/FileSystemHelper.php b/src/Spout/Common/Helper/FileSystemHelper.php index d7ca64f..6186822 100644 --- a/src/Spout/Common/Helper/FileSystemHelper.php +++ b/src/Spout/Common/Helper/FileSystemHelper.php @@ -63,7 +63,7 @@ class FileSystemHelper $filePath = $parentFolderPath . '/' . $fileName; $wasCreationSuccessful = file_put_contents($filePath, $fileContents); - if (!$wasCreationSuccessful) { + if ($wasCreationSuccessful === false) { throw new IOException('Unable to create file: ' . $filePath); } diff --git a/src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php b/src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php index 5c8fb46..75f8989 100644 --- a/src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php +++ b/src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php @@ -154,7 +154,8 @@ class SharedStringsHelper $readError = libxml_get_last_error(); if ($readError !== false) { - throw new IOException("The sharedStrings.xml file is invalid and cannot be read. [{$readError->message}]"); + $readErrorMessage = trim($readError->message); + throw new IOException("The sharedStrings.xml file is invalid and cannot be read. [{$readErrorMessage}]"); } // reset the setting to display XML warnings/errors diff --git a/src/Spout/Reader/XLSX/RowIterator.php b/src/Spout/Reader/XLSX/RowIterator.php index e96898f..6fc1dde 100644 --- a/src/Spout/Reader/XLSX/RowIterator.php +++ b/src/Spout/Reader/XLSX/RowIterator.php @@ -131,12 +131,19 @@ class RowIterator implements IteratorInterface * * @return void * @throws \Box\Spout\Reader\Exception\SharedStringNotFoundException If a shared string was not found + * @throws \Box\Spout\Common\Exception\IOException If unable to read the sheet data XML */ public function next() { $isInsideRowTag = false; $rowData = []; + // Use internal errors to avoid displaying lots of warning messages in case of invalid file + // For instance on HHVM, XMLReader->open() won't fail when trying to read a unexisting file within a zip... + // But the XMLReader->read() will fail! + libxml_clear_errors(); + libxml_use_internal_errors(true); + while ($this->xmlReader->read()) { if ($this->xmlReader->nodeType == \XMLReader::ELEMENT && $this->xmlReader->name === self::XML_NODE_DIMENSION) { // Read dimensions of the sheet @@ -180,6 +187,12 @@ class RowIterator implements IteratorInterface } } + $readError = libxml_get_last_error(); + if ($readError !== false) { + $readErrorMessage = trim($readError->message); + throw new IOException("The {$this->sheetDataXMLFilePath} file cannot be read. [{$readErrorMessage}]"); + } + $this->rowDataBuffer = $rowData; } diff --git a/tests/Spout/Reader/CSV/ReaderTest.php b/tests/Spout/Reader/CSV/ReaderTest.php index 922c61b..932633f 100644 --- a/tests/Spout/Reader/CSV/ReaderTest.php +++ b/tests/Spout/Reader/CSV/ReaderTest.php @@ -54,6 +54,25 @@ class ReaderTest extends \PHPUnit_Framework_TestCase $reader->open($resourcePath); } + /** + * @expectedException \Box\Spout\Common\Exception\IOException + * + * @return void + */ + public function testOpenShouldThrowExceptionIfCannotOpenFile() + { + $helperStub = $this->getMockBuilder('\Box\Spout\Common\Helper\GlobalFunctionsHelper') + ->setMethods(['fopen']) + ->getMock(); + $helperStub->method('fopen')->willReturn(false); + + $resourcePath = $this->getResourcePath('csv_standard.csv'); + + $reader = ReaderFactory::create(Type::CSV); + $reader->setGlobalFunctionsHelper($helperStub); + $reader->open($resourcePath); + } + /** * @return void @@ -161,6 +180,50 @@ class ReaderTest extends \PHPUnit_Framework_TestCase $this->assertEquals($expectedRows, $allRows); } + /** + * @return void + */ + public function testReadMultipleTimesShouldRewindReader() + { + $allRows = []; + $resourcePath = $this->getResourcePath('csv_standard.csv'); + + $reader = ReaderFactory::create(Type::CSV); + $reader->open($resourcePath); + + foreach ($reader->getSheetIterator() as $sheet) { + // do nothing + } + + foreach ($reader->getSheetIterator() as $sheet) { + foreach ($sheet->getRowIterator() as $row) { + $allRows[] = $row; + break; + } + + foreach ($sheet->getRowIterator() as $row) { + $allRows[] = $row; + break; + } + } + + foreach ($reader->getSheetIterator() as $sheet) { + foreach ($sheet->getRowIterator() as $row) { + $allRows[] = $row; + break; + } + } + + $reader->close(); + + $expectedRows = [ + ['csv--11', 'csv--12', 'csv--13'], + ['csv--11', 'csv--12', 'csv--13'], + ['csv--11', 'csv--12', 'csv--13'], + ]; + $this->assertEquals($expectedRows, $allRows); + } + /** * @param string $fileName * @param string|void $fieldDelimiter diff --git a/tests/Spout/Reader/XLSX/ReaderTest.php b/tests/Spout/Reader/XLSX/ReaderTest.php index 9643d54..8c8376b 100644 --- a/tests/Spout/Reader/XLSX/ReaderTest.php +++ b/tests/Spout/Reader/XLSX/ReaderTest.php @@ -6,6 +6,7 @@ use Box\Spout\Common\Exception\IOException; use Box\Spout\Common\Type; use Box\Spout\Reader\ReaderFactory; use Box\Spout\TestUsingResource; +use Symfony\Component\Config\Definition\Exception\Exception; /** * Class ReaderTest @@ -24,6 +25,7 @@ class ReaderTest extends \PHPUnit_Framework_TestCase return [ ['/path/to/fake/file.xlsx'], ['file_with_no_sheets_in_content_types.xlsx'], + ['file_with_sheet_xml_not_matching_content_types.xlsx'], ['file_corrupted.xlsx'], ]; } @@ -37,7 +39,8 @@ class ReaderTest extends \PHPUnit_Framework_TestCase */ public function testReadShouldThrowException($filePath) { - $this->getAllRowsForFile($filePath); + // using @ to prevent warnings/errors from being displayed + @$this->getAllRowsForFile($filePath); } /** @@ -240,7 +243,8 @@ class ReaderTest extends \PHPUnit_Framework_TestCase $startTime = microtime(true); try { - $this->getAllRowsForFile($fileName); + // using @ to prevent warnings/errors from being displayed + @$this->getAllRowsForFile($fileName); $this->fail('An exception should have been thrown'); } catch (IOException $exception) { $duration = microtime(true) - $startTime; @@ -275,6 +279,60 @@ class ReaderTest extends \PHPUnit_Framework_TestCase $this->assertEquals($expectedRows, $allRows); } + /** + * @return void + */ + public function testReadMultipleTimesShouldRewindReader() + { + $allRows = []; + $resourcePath = $this->getResourcePath('two_sheets_with_inline_strings.xlsx'); + + $reader = ReaderFactory::create(Type::XLSX); + $reader->open($resourcePath); + + foreach ($reader->getSheetIterator() as $sheet) { + // do nothing + } + + foreach ($reader->getSheetIterator() as $sheet) { + // this loop should only add the first row of the first sheet + foreach ($sheet->getRowIterator() as $row) { + $allRows[] = $row; + break; + } + + // this loop should rewind the iterator and restart reading from the 1st row again + // therefore, it should only add the first row of the first sheet + foreach ($sheet->getRowIterator() as $row) { + $allRows[] = $row; + break; + } + + // not reading any more sheets + break; + } + + foreach ($reader->getSheetIterator() as $sheet) { + // this loop should only add the first row of the current sheet + foreach ($sheet->getRowIterator() as $row) { + $allRows[] = $row; + break; + } + + // not breaking, so we keep reading the next sheets + } + + $reader->close(); + + $expectedRows = [ + ['s1 - A1', 's1 - B1', 's1 - C1', 's1 - D1', 's1 - E1'], + ['s1 - A1', 's1 - B1', 's1 - C1', 's1 - D1', 's1 - E1'], + ['s1 - A1', 's1 - B1', 's1 - C1', 's1 - D1', 's1 - E1'], + ['s2 - A1', 's2 - B1', 's2 - C1', 's2 - D1', 's2 - E1'], + ]; + $this->assertEquals($expectedRows, $allRows); + } + /** * @param string $fileName * @return array All the read rows the given file diff --git a/tests/resources/xlsx/file_with_sheet_xml_not_matching_content_types.xlsx b/tests/resources/xlsx/file_with_sheet_xml_not_matching_content_types.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..f33c9d7817f057af2fa1d0efe2b717be4c7e2953 GIT binary patch literal 3757 zcmai12{@Gf79ZKBFxDb5woul=$X3>|FXKarB7=#s%*ei%iSfxUMOpfk)Fs={ib~^C z3SYL7?Af#jQK%Bt8=X7vbUo_VDf>n?g+Q&{N(9@$TRm+$J<9a)d@68#M@I1jP+>B+cz6Z&4 ziK(05C5!b}DL=(c8+RiRR6Ln4ywcF@KWo4H(DzgU?%>GH5N zLMBl2U2kBge`4s|v+$X4JFmeDGy6?gG>>-3wm8EeUnf4h!R%l8^Bq4Xc&sIDrXutk z>l2kw0;kcud(-g=1{?FP$C?lPM~$Orjzl>?MP`MRpVzj3edYNK9ig7&JlrH!GL%bm zow0~6?ocg{Ya-0P-%H+tB91-2qVZvp-~1=ZAW&-Af*!!@)?omCUGPSG_{jc}*-S)$ zjzH-M@Xm1`2*k7{BeNk{m||*N17jmh2HvX0)8s8a_Hwo#;KLc`WbTdg z+D#kRALQOD;I%4H!))8dP+nde#haB>eQlo{8)BO2yUsg1g|Au&c@cb|y%#yeAQ%oC zgBVIu7N0zOI5Kt4ulip8B;~fAUxbASs$ir{Pf`;p^rf`z)f)CiYd(?Gr>E_@z>;qt za9wuA@|aFP`1JmL?ghyDAhgMXxIs~4jou{&iMrBv-f%c`6Y1uv3Jq}?dpjyZl9mD7 zvbt|v{E}b)x;E{y{>$J|oK0YgbzMjHr}!|FkBa2VzwcDk1;zE0N;ORh37?%?PH7yr zdr?8VvqI#gz%txGm{Y*RzCBbYoHu$SR?lQRBuS8qFm1ZZ%Cac^ky4ucq~hbG5hthn zuyllzzEmoxsPk1Cxll&+t63w7)L%K|R$SrC*jUhG_2)%8Q3r(V4UFVG`B4kKQD~pu zY>|?A4sBq0esQr8>eeWK<+C0<(}7S9(^>_TWI0QA8JQ5pkTsu^GU zOKt9{iqDJPKiUvWkg<8`@bp7!mx8v@s=l}PTROhU*7WoI|C*B@zPe6tb$N@$y>&$k zuo&p^e+mx}_}zV#VDq;p19&6gzVdGB*%S-FdAmE}aPAu!`FVXC+(S7YkC{SYM-ZPhw;U?T>qH zMWR*OeBXm|Bw`eZwu%g{ZIwe)SD4+ZOanipe|l0yLi-Ac5LW+zH-y~14=Ic2tz?6Y z#)|cDTw|?^?y??b4V?C~eOYAPZ8;{{OgQDJKF(*P@y6SV@xJYy!kSRVowfO|g*UbF z01GdGXJfbkr1`j_ywT23%ZIkVZ@G)zm_w1ZPIO{-YSUG`T5HVHKR~#Fs@cMllZwCG{?n7EY zqp;zl`xgOZl9f~q9sd56+~3_uY(fbgG;R!YdKKPhu%U5)H2YR2uhCm{EqKMfn{W7eNI$TN2jdR9Y%5OU6z@`x#$ zTy$qn#iUq(l+z1NaAVG0%#*b1ou@25=UJdEaJ8?UU!&Q{G3&)t(p!LAMSy44O^&x; zmH0$0Gm<8=?xH~#YVcH6E>q;)+-@rayQh2(@01%J9#$hpKk0&|S$z5uzvel%yzU=X zcU>*V)p7hUd#k>r;Di_mf92GLJNB!1cRl+ZdrSV@et!m?E#=PE09 zx_z)dqs;Bq`e-7hQ~5xL<7APKXH!ww5or}=ObQDzUU5n7?{!Dp%g9Q~zF#r#Z8i{U z%s#GYwD0aoX0UQ1zz^W25DWqd?~(lhF~s_#JTT5?D9<1L{sy_*>?(}w^-Kt7Z^V4~ z#8SGPG3u~q7|j9=FPTNSJh#xSw!L)#JD9yiB z%wFZJ(nB_6cQU03^Y{*yH~4np8KkdymJl~;07f{)_d<`UGZgYUlIe?Yv36*9um3&q znf{c(5)6M1o^r%Q#|3G4UR>0gG^XtXoo-$tipGSn88zQ}sX$RwwkU11maz6;TYX#>K4ktJ$*MsRq9Q1^FRNlgPmA>mmd*ta5Mw6=f*-Tgz_^Q3q*};0R z;}9_UiZUJb79oA#cm0rE$XmS~Iq75Of~s6zjC7VHW_=jnMLIk?OoTm7>g!}l_E?FY z?^)7rh2HU_uSXZsGMd%};>WqojcI@;@!uC{AamQ!Z`Nt5g1t4eu^dtl8mNX}qi3hl z-OC~s6SIZ+F?;@(v5l>YdTs$xZQHSS+S|QlQDLvQ!+!2uRHK`#?e4{eiY@>q@RreE zYYY{>m!OS_O+7QfN6U}-{ma|EfSo~d(EkAZp48nFooYvAJ8XN?+Qso^xVvW|RgXT9 z-``C}YQ&owc8?>f1_s7W4O_!$H*|NGr9$0+z5*EE?miW~m$;2~MLqTak3Yd%&FdHG jn?CPuUsUY1J=othGjn75AJq>5K|p7LG;3^p1cCkoa=d>q literal 0 HcmV?d00001