diff --git a/src/Spout/Common/Helper/GlobalFunctionsHelper.php b/src/Spout/Common/Helper/GlobalFunctionsHelper.php index c5d6e31..7eb66b4 100644 --- a/src/Spout/Common/Helper/GlobalFunctionsHelper.php +++ b/src/Spout/Common/Helper/GlobalFunctionsHelper.php @@ -203,6 +203,18 @@ class GlobalFunctionsHelper return (strpos($path, 'zip://') === 0); } + /** + * Wrapper around global function dirname() + * @see dirname() + * + * @param string $filePath + * @return string + */ + public function dirname($filePath) + { + return dirname($filePath); + } + /** * Wrapper around global function feof() * @see feof() diff --git a/src/Spout/Writer/AbstractWriter.php b/src/Spout/Writer/AbstractWriter.php index 49939ee..af97ead 100644 --- a/src/Spout/Writer/AbstractWriter.php +++ b/src/Spout/Writer/AbstractWriter.php @@ -4,6 +4,8 @@ namespace Box\Spout\Writer; use Box\Spout\Common\Exception\IOException; use Box\Spout\Common\Exception\InvalidArgumentException; +use Box\Spout\Common\Exception\SpoutException; +use Box\Spout\Common\Helper\FileSystemHelper; use Box\Spout\Writer\Exception\WriterAlreadyOpenedException; use Box\Spout\Writer\Exception\WriterNotOpenedException; use Box\Spout\Writer\Style\StyleBuilder; @@ -199,13 +201,23 @@ abstract class AbstractWriter implements WriterInterface * @return AbstractWriter * @throws \Box\Spout\Writer\Exception\WriterNotOpenedException If this function is called before opening the writer * @throws \Box\Spout\Common\Exception\IOException If unable to write data + * @throws \Box\Spout\Common\Exception\SpoutException If anything else goes wrong while writing data */ public function addRow(array $dataRow) { if ($this->isWriterOpened) { // empty $dataRow should not add an empty line if (!empty($dataRow)) { - $this->addRowToWriter($dataRow, $this->rowStyle); + try { + $this->addRowToWriter($dataRow, $this->rowStyle); + } catch (SpoutException $e) { + // if an exception occurs while writing data, + // close the writer and remove all files created so far. + $this->closeAndAttemptToCleanupAllFiles(); + + // re-throw the exception to alert developers of the error + throw $e; + } } } else { throw new WriterNotOpenedException('The writer needs to be opened before adding row.'); @@ -346,5 +358,24 @@ abstract class AbstractWriter implements WriterInterface $this->isWriterOpened = false; } + + /** + * Closes the writer and attempts to cleanup all files that were + * created during the writing process (temp files & final file). + * + * @return void + */ + private function closeAndAttemptToCleanupAllFiles() + { + // close the writer, which should remove all temp files + $this->close(); + + // remove output file if it was created + if ($this->globalFunctionsHelper->file_exists($this->outputFilePath)) { + $outputFolder = $this->globalFunctionsHelper->dirname($this->outputFilePath); + $fileSystemHelper = new FileSystemHelper($outputFolder); + $fileSystemHelper->deleteFile($this->outputFilePath); + } + } } diff --git a/tests/Spout/TestUsingResource.php b/tests/Spout/TestUsingResource.php index 17c37b5..2b200d9 100644 --- a/tests/Spout/TestUsingResource.php +++ b/tests/Spout/TestUsingResource.php @@ -18,6 +18,9 @@ trait TestUsingResource /** @var string Path to the test resources folder, that does not have writing permissions */ private $generatedUnwritableResourcesPath = 'tests/resources/generated/unwritable'; + /** @var string Path to the test temp folder */ + private $tempFolderPath = 'tests/resources/generated/temp'; + /** * @param string $resourceName * @return string|null Path of the resource who matches the given name or null if resource not found @@ -86,6 +89,48 @@ trait TestUsingResource } } + /** + * @return string Path of the temp folder + */ + protected function getTempFolderPath() + { + return realpath($this->tempFolderPath); + } + + /** + * @return void + */ + protected function recreateTempFolder() + { + if (file_exists($this->tempFolderPath)) { + $this->deleteFolderRecursively($this->tempFolderPath); + } + + mkdir($this->tempFolderPath, 0777, true); + } + + /** + * @param string $folderPath + * @return void + */ + private function deleteFolderRecursively($folderPath) + { + $itemIterator = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($folderPath, \RecursiveDirectoryIterator::SKIP_DOTS), + \RecursiveIteratorIterator::CHILD_FIRST + ); + + foreach ($itemIterator as $item) { + if ($item->isDir()) { + rmdir($item->getPathname()); + } else { + unlink($item->getPathname()); + } + } + + rmdir($folderPath); + } + /** * @return bool Whether the OS on which PHP is installed is Windows */ diff --git a/tests/Spout/Writer/ODS/WriterTest.php b/tests/Spout/Writer/ODS/WriterTest.php index 43383e4..7c4b396 100644 --- a/tests/Spout/Writer/ODS/WriterTest.php +++ b/tests/Spout/Writer/ODS/WriterTest.php @@ -2,6 +2,7 @@ namespace Box\Spout\Writer\ODS; +use Box\Spout\Common\Exception\SpoutException; use Box\Spout\Common\Type; use Box\Spout\Reader\Wrapper\XMLReader; use Box\Spout\TestUsingResource; @@ -91,6 +92,39 @@ class WriterTest extends \PHPUnit_Framework_TestCase $this->writeToODSFile($dataRows, $fileName); } + /** + * @return void + */ + public function testAddRowShouldCleanupAllFilesIfExceptionIsThrown() + { + $fileName = 'test_add_row_should_cleanup_all_files_if_exception_thrown.ods'; + $dataRows = [ + ['wrong'], + [new \stdClass()], + ]; + + $this->createGeneratedFolderIfNeeded($fileName); + $resourcePath = $this->getGeneratedResourcePath($fileName); + + $this->recreateTempFolder(); + $tempFolderPath = $this->getTempFolderPath(); + + /** @var \Box\Spout\Writer\ODS\Writer $writer */ + $writer = WriterFactory::create(Type::ODS); + $writer->setTempFolder($tempFolderPath); + $writer->openToFile($resourcePath); + + try { + $writer->addRows($dataRows); + $this->fail('Exception should have been thrown'); + } catch (SpoutException $e) { + $this->assertFalse(file_exists($fileName), 'Output file should have been deleted'); + + $numFiles = iterator_count(new \FilesystemIterator($tempFolderPath, \FilesystemIterator::SKIP_DOTS)); + $this->assertEquals(0, $numFiles, 'All temp files should have been deleted'); + } + } + /** * @return void */ diff --git a/tests/Spout/Writer/XLSX/WriterTest.php b/tests/Spout/Writer/XLSX/WriterTest.php index e74cd9c..cc8daca 100644 --- a/tests/Spout/Writer/XLSX/WriterTest.php +++ b/tests/Spout/Writer/XLSX/WriterTest.php @@ -2,6 +2,7 @@ namespace Box\Spout\Writer\XLSX; +use Box\Spout\Common\Exception\SpoutException; use Box\Spout\Common\Type; use Box\Spout\TestUsingResource; use Box\Spout\Writer\WriterFactory; @@ -104,6 +105,39 @@ class WriterTest extends \PHPUnit_Framework_TestCase $this->writeToXLSXFile($dataRows, $fileName); } + /** + * @return void + */ + public function testAddRowShouldCleanupAllFilesIfExceptionIsThrown2() + { + $fileName = 'test_add_row_should_cleanup_all_files_if_exception_thrown.xlsx'; + $dataRows = [ + ['wrong'], + [new \stdClass()], + ]; + + $this->createGeneratedFolderIfNeeded($fileName); + $resourcePath = $this->getGeneratedResourcePath($fileName); + + $this->recreateTempFolder(); + $tempFolderPath = $this->getTempFolderPath(); + + /** @var \Box\Spout\Writer\XLSX\Writer $writer */ + $writer = WriterFactory::create(Type::XLSX); + $writer->setTempFolder($tempFolderPath); + $writer->openToFile($resourcePath); + + try { + $writer->addRows($dataRows); + $this->fail('Exception should have been thrown'); + } catch (SpoutException $e) { + $this->assertFalse(file_exists($fileName), 'Output file should have been deleted'); + + $numFiles = iterator_count(new \FilesystemIterator($tempFolderPath, \FilesystemIterator::SKIP_DOTS)); + $this->assertEquals(0, $numFiles, 'All temp files should have been deleted'); + } + } + /** * @return void */