diogo
/
symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

merged branch jakzal/filesystem-dump-file (PR #7820) 

This PR was merged into the master branch.

Discussion
----------

[Filesystem] Made sure Filesystem::dumpFile() overwrites an existing file

#7753 introduced a change in behaviour. Before, ConfigCache simply used the ``rename()`` function to save a file. It was replaced by ``Filesystem::dumpFile()`` which internally uses ``Filesystem::rename()``. The later checks if file already exists and throws an exception if it exists.

This PR makes sure that ``Filesystem::dumpFile()`` removes the file before creating a new one.

Alternatively, we could introduce a third parameter to the ``Filesystem::rename()`` which would allow to overwrite the target file.

Before #7752:
* [CompilerDebugDumpPass](d100ffaf76/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CompilerDebugDumpPass.php (L23)) uses ConfigCache
* [ConfigCache](d100ffaf76/src/Symfony/Component/Config/ConfigCache.php (L103)) uses the [rename()](http://uk1.php.net/rename) function (which overwrites a file if it exists)

After #7752:
* [CompilerDebugDumpPass](ad47bc4738/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CompilerDebugDumpPass.php (L23)) uses the ``Filesystem::dumpFile()``
* [Filesystem::dumpFile()](ad47bc4738/src/Symfony/Component/Filesystem/Filesystem.php (L458)) uses the [Filesystem::rename()](ad47bc4738/src/Symfony/Component/Filesystem/Filesystem.php (L239)) (which throws an exception if file exists)

| Q | 	A
| ------------- | ---
| Bug fix?	| yes
| New feature?	| no
| BC breaks?	| no
| Deprecations?	| no
| Tests pass?	| yes
| Fixed tickets	| #7816
| License | 	MIT
| Doc PR	| ~

Commits
-------

4f4ec76 [Filesystem] Made sure Filesystem::dumpFile() overwrites an existing file.
This commit is contained in:
Fabien Potencier 2013-04-24 11:28:15 +02:00
parent ae88c02236 4f4ec76ba4
commit 756d51929e
2 changed files with 31 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/Symfony/Component/Filesystem/Filesystem.php
View File

@ -224,16 +224,17 @@ class Filesystem
/**
* Renames a file.
*
* @param string $origin The origin filename
* @param string $target The new filename
* @param string $origin The origin filename
* @param string $target The new filename
* @param Boolean $overwrite Whether to overwrite the target if it already exists
*
* @throws IOException When target file already exists
* @throws IOException When origin cannot be renamed
*/
public function rename($origin, $target)
public function rename($origin, $target, $overwrite = false)
{
// we check that target does not exist
if (is_readable($target)) {
if (!$overwrite && is_readable($target)) {
throw new IOException(sprintf('Cannot rename because the target "%s" already exist.', $target));
}
@ -452,7 +453,7 @@ class Filesystem
throw new IOException(sprintf('Failed to write file "%s".', $filename));
}
$this->rename($tmpFile, $filename);
$this->rename($tmpFile, $filename, true);
$this->chmod($filename, $mode);
}
}

25
src/Symfony/Component/Filesystem/Tests/FilesystemTest.php
View File

@ -639,6 +639,20 @@ class FilesystemTest extends \PHPUnit_Framework_TestCase
$this->filesystem->rename($file, $newPath);
}
public function testRenameOverwritesTheTargetIfItAlreadyExists()
{
$file = $this->workspace.DIRECTORY_SEPARATOR.'file';
$newPath = $this->workspace.DIRECTORY_SEPARATOR.'new_file';
touch($file);
touch($newPath);
$this->filesystem->rename($file, $newPath, true);
$this->assertFileNotExists($file);
$this->assertFileExists($newPath);
}
/**
* @expectedException \Symfony\Component\Filesystem\Exception\IOException
*/
@ -894,6 +908,17 @@ class FilesystemTest extends \PHPUnit_Framework_TestCase
$this->assertEquals(753, $this->getFilePermissions($filename));
}
public function testDumpFileOverwritesAnExistingFile()
{
$filename = $this->workspace.DIRECTORY_SEPARATOR.'foo.txt';
file_put_contents($filename, 'FOO BAR');
$this->filesystem->dumpFile($filename, 'bar');
$this->assertFileExists($filename);
$this->assertSame('bar', file_get_contents($filename));
}
/**
* Returns file permissions as three digits (i.e. 755)
*