mcndave/spout
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Temp files should be deleted when an exception is thrown

Browse Source 
If an exception is thrown while writing data, instead of letting the developer handle this situation gracefully, Spout can attempt to delete all the temporary files that were created so far, as well as the output file as it won't be completed and therefore corrupted.
This commit is contained in:
Adrien Loison 2016-10-11 14:55:41 -07:00
parent 442a9837f1
commit b684acb14b
5 changed files with 157 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/Spout/Common/Helper/GlobalFunctionsHelper.php
View File

@ -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()

33
src/Spout/Writer/AbstractWriter.php
View File

@ -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);
}
}
}

45
tests/Spout/TestUsingResource.php
View File

@ -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
*/

34
tests/Spout/Writer/ODS/WriterTest.php
View File

@ -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
*/

34
tests/Spout/Writer/XLSX/WriterTest.php
View File

@ -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
*/