merged branch jakzal/FilesystemTests (PR #3811)

Commits
-------

100e97e [Filesystem] Fixed warnings in makePathRelative().
f5f5c21 [Filesystem] Fixed typos in the docblocks.
d4243a2 [Filesystem] Fixed a bug in remove being unable to remove symlinks to unexisting file or directory.
11a676d [Filesystem] Added unit tests for mirror method.
8c94069 [Filesystem] Added unit tests for isAbsolutePath method.
2ee4b88 [Filesystem] Added unit tests for makePathRelative method.
21860cb [Filesystem] Added unit tests for symlink method.
a041feb [Filesystem] Added unit tests for rename method.
8071859 [Filesystem] Added unit tests for chmod method.
bba0080 [Filesystem] Added unit tests for remove method.
8e861b7 [Filesystem] Introduced workspace directory to limit complexity of tests.
a91e200 [Filesystem] Added unit tests for touch method.
7e297db [Filesystem] Added unit tests for mkdir method.
6ac5486 [Filesystem] Added unit tests for copy method.
1c833e7 [Filesystem] Added missing docblock comment.

Discussion
----------

[Filesystem] Fixed a bug in remove() being unable to unlink broken symlinks

While working on test coverage for Filesystem class I discovered a bug in remove() method.

Before removing a file a check is made if it exists:

    if (!file_exists($file)) {
        continue;
    }

Problem is [file_exists()](http://php.net/file_exists) returns false if link's target file doesn't exist. Therefore remove() will fail to delete a directory containing a broken link. Solution is to handle links a bit different:

    if (!file_exists($file) && !is_link($file)) {
        continue;
    }

Additionally, this PR improves test coverage of Filesystem component.

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes

---------------------------------------------------------------------------

by cordoval at 2012-04-07T00:55:59Z

✌.|•͡˘‿•͡˘|.✌

---------------------------------------------------------------------------

by fabpot at 2012-04-07T06:12:34Z

Tests do not pass for me:

    PHPUnit 3.6.10 by Sebastian Bergmann.

    Configuration read from /Users/fabien/work/symfony/git/symfony/phpunit.xml.dist

    .........................EE.......

    Time: 0 seconds, Memory: 5.25Mb

    There were 2 errors:

    1) Symfony\Component\Filesystem\Tests\FilesystemTest::testMakePathRelative with data set #0 ('/var/lib/symfony/src/Symfony/', '/var/lib/symfony/src/Symfony/Component', '../')
    Uninitialized string offset: 29

    .../rc/Symfony/Component/Filesystem/Filesystem.php:183
    .../rc/Symfony/Component/Filesystem/Tests/FilesystemTest.php:434

    2) Symfony\Component\Filesystem\Tests\FilesystemTest::testMakePathRelative with data set #1 ('var/lib/symfony/', 'var/lib/symfony/src/Symfony/Component', '../../../')
    Uninitialized string offset: 16

    .../rc/Symfony/Component/Filesystem/Filesystem.php:183
    .../rc/Symfony/Component/Filesystem/Tests/FilesystemTest.php:434

    FAILURES!
    Tests: 34, Assertions: 67, Errors: 2.

---------------------------------------------------------------------------

by jakzal at 2012-04-07T07:26:15Z

Sorry for this. For some reason my PHP error reporting level was to low to catch this...

Should be fixed now but I needed to modify the makePathRelative() (this bug existed before).
This commit is contained in:
Fabien Potencier 2012-04-07 10:18:55 +02:00
commit 13aa515d65
2 changed files with 534 additions and 6 deletions

View File

@ -69,7 +69,7 @@ class Filesystem
/**
* Creates empty files.
*
* @param string|array|\Traversable $files A filename, an array of files, or a \Traversable instance to remove
* @param string|array|\Traversable $files A filename, an array of files, or a \Traversable instance to create
*/
public function touch($files)
{
@ -88,7 +88,7 @@ class Filesystem
$files = iterator_to_array($this->toIterator($files));
$files = array_reverse($files);
foreach ($files as $file) {
if (!file_exists($file)) {
if (!file_exists($file) && !is_link($file)) {
continue;
}
@ -105,7 +105,7 @@ class Filesystem
/**
* Change mode for an array of files or directories.
*
* @param string|array|\Traversable $files A filename, an array of files, or a \Traversable instance to remove
* @param string|array|\Traversable $files A filename, an array of files, or a \Traversable instance to change mode
* @param integer $mode The new mode
* @param integer $umask The mode mask (octal)
*/
@ -171,8 +171,8 @@ class Filesystem
/**
* Given an existing path, convert it to a path relative to a given starting path
*
* @var string Absolute path of target
* @var string Absolute path where traversal begins
* @param string $endPath Absolute path of target
* @param string $startPath Absolute path where traversal begins
*
* @return string Path of target relative to starting path
*/
@ -180,7 +180,7 @@ class Filesystem
{
// Find for which character the the common path stops
$offset = 0;
while ($startPath[$offset] === $endPath[$offset]) {
while (isset($startPath[$offset]) && isset($endPath[$offset]) && $startPath[$offset] === $endPath[$offset]) {
$offset++;
}
@ -264,6 +264,11 @@ class Filesystem
return false;
}
/**
* @param mixed $files
*
* @return \Traversable
*/
private function toIterator($files)
{
if (!$files instanceof \Traversable) {

View File

@ -0,0 +1,523 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\Filesystem\Tests;
use Symfony\Component\Filesystem\Filesystem;
/**
* Test class for Filesystem.
*/
class FilesystemTest extends \PHPUnit_Framework_TestCase
{
/**
* @var string $workspace
*/
private $workspace = null;
/**
* @var \Symfony\Component\Filesystem\Filesystem $filesystem
*/
private $filesystem = null;
public function setUp()
{
$this->filesystem = new Filesystem();
$this->workspace = sys_get_temp_dir().DIRECTORY_SEPARATOR.time().rand(0, 1000);
mkdir($this->workspace, 0777, true);
}
public function tearDown()
{
$this->clean($this->workspace);
}
/**
* @param string $file
*/
private function clean($file)
{
if (is_dir($file) && !is_link($file)) {
$dir = new \FilesystemIterator($file);
foreach ($dir as $childFile) {
$this->clean($childFile);
}
rmdir($file);
} else {
unlink($file);
}
}
public function testCopyCreatesNewFile()
{
$sourceFilePath = $this->workspace.DIRECTORY_SEPARATOR.'copy_source_file';
$targetFilePath = $this->workspace.DIRECTORY_SEPARATOR.'copy_target_file';
file_put_contents($sourceFilePath, 'SOURCE FILE');
$this->filesystem->copy($sourceFilePath, $targetFilePath);
$this->assertFileExists($targetFilePath);
$this->assertEquals('SOURCE FILE', file_get_contents($targetFilePath));
}
public function testCopyOverridesExistingFileIfModified()
{
$sourceFilePath = $this->workspace.DIRECTORY_SEPARATOR.'copy_source_file';
$targetFilePath = $this->workspace.DIRECTORY_SEPARATOR.'copy_target_file';
file_put_contents($sourceFilePath, 'SOURCE FILE');
file_put_contents($targetFilePath, 'TARGET FILE');
touch($targetFilePath, time() - 1000);
$this->filesystem->copy($sourceFilePath, $targetFilePath);
$this->assertFileExists($targetFilePath);
$this->assertEquals('SOURCE FILE', file_get_contents($targetFilePath));
}
public function testCopyDoesNotOverrideExistingFileByDefault()
{
$sourceFilePath = $this->workspace.DIRECTORY_SEPARATOR.'copy_source_file';
$targetFilePath = $this->workspace.DIRECTORY_SEPARATOR.'copy_target_file';
file_put_contents($sourceFilePath, 'SOURCE FILE');
file_put_contents($targetFilePath, 'TARGET FILE');
// make sure both files have the same modification time
$modificationTime = time() - 1000;
touch($sourceFilePath, $modificationTime);
touch($targetFilePath, $modificationTime);
$this->filesystem->copy($sourceFilePath, $targetFilePath);
$this->assertFileExists($targetFilePath);
$this->assertEquals('TARGET FILE', file_get_contents($targetFilePath));
}
public function testCopyOverridesExistingFileIfForced()
{
$sourceFilePath = $this->workspace.DIRECTORY_SEPARATOR.'copy_source_file';
$targetFilePath = $this->workspace.DIRECTORY_SEPARATOR.'copy_target_file';
file_put_contents($sourceFilePath, 'SOURCE FILE');
file_put_contents($targetFilePath, 'TARGET FILE');
// make sure both files have the same modification time
$modificationTime = time() - 1000;
touch($sourceFilePath, $modificationTime);
touch($targetFilePath, $modificationTime);
$this->filesystem->copy($sourceFilePath, $targetFilePath, true);
$this->assertFileExists($targetFilePath);
$this->assertEquals('SOURCE FILE', file_get_contents($targetFilePath));
}
public function testCopyCreatesTargetDirectoryIfItDoesNotExist()
{
$sourceFilePath = $this->workspace.DIRECTORY_SEPARATOR.'copy_source_file';
$targetFileDirectory = $this->workspace.DIRECTORY_SEPARATOR.'directory';
$targetFilePath = $targetFileDirectory.DIRECTORY_SEPARATOR.'copy_target_file';
file_put_contents($sourceFilePath, 'SOURCE FILE');
$this->filesystem->copy($sourceFilePath, $targetFilePath);
$this->assertTrue(is_dir($targetFileDirectory));
$this->assertFileExists($targetFilePath);
$this->assertEquals('SOURCE FILE', file_get_contents($targetFilePath));
}
public function testMkdirCreatesDirectoriesRecursively()
{
$directory = $this->workspace
.DIRECTORY_SEPARATOR.'directory'
.DIRECTORY_SEPARATOR.'sub_directory';
$result = $this->filesystem->mkdir($directory);
$this->assertTrue($result);
$this->assertTrue(is_dir($directory));
}
public function testMkdirCreatesDirectoriesFromArray()
{
$basePath = $this->workspace.DIRECTORY_SEPARATOR;
$directories = array(
$basePath.'1', $basePath.'2', $basePath.'3'
);
$result = $this->filesystem->mkdir($directories);
$this->assertTrue($result);
$this->assertTrue(is_dir($basePath.'1'));
$this->assertTrue(is_dir($basePath.'2'));
$this->assertTrue(is_dir($basePath.'3'));
}
public function testMkdirCreatesDirectoriesFromTraversableObject()
{
$basePath = $this->workspace.DIRECTORY_SEPARATOR;
$directories = new \ArrayObject(array(
$basePath.'1', $basePath.'2', $basePath.'3'
));
$result = $this->filesystem->mkdir($directories);
$this->assertTrue($result);
$this->assertTrue(is_dir($basePath.'1'));
$this->assertTrue(is_dir($basePath.'2'));
$this->assertTrue(is_dir($basePath.'3'));
}
public function testMkdirCreatesDirectoriesEvenIfItFailsToCreateOneOfThem()
{
$basePath = $this->workspace.DIRECTORY_SEPARATOR;
$directories = array(
$basePath.'1', $basePath.'2', $basePath.'3'
);
// create a file to make that directory cannot be created
file_put_contents($basePath.'2', '');
$result = $this->filesystem->mkdir($directories);
$this->assertFalse($result);
$this->assertTrue(is_dir($basePath.'1'));
$this->assertFalse(is_dir($basePath.'2'));
$this->assertTrue(is_dir($basePath.'3'));
}
public function testTouchCreatesEmptyFile()
{
$file = $this->workspace.DIRECTORY_SEPARATOR.'1';
$this->filesystem->touch($file);
$this->assertFileExists($file);
}
public function testTouchCreatesEmptyFilesFromArray()
{
$basePath = $this->workspace.DIRECTORY_SEPARATOR;
$files = array(
$basePath.'1', $basePath.'2', $basePath.'3'
);
$this->filesystem->touch($files);
$this->assertFileExists($basePath.'1');
$this->assertFileExists($basePath.'2');
$this->assertFileExists($basePath.'3');
}
public function testTouchCreatesEmptyFilesFromTraversableObject()
{
$basePath = $this->workspace.DIRECTORY_SEPARATOR;
$files = new \ArrayObject(array(
$basePath.'1', $basePath.'2', $basePath.'3'
));
$this->filesystem->touch($files);
$this->assertFileExists($basePath.'1');
$this->assertFileExists($basePath.'2');
$this->assertFileExists($basePath.'3');
}
public function testRemoveCleansFilesAndDirectoriesIteratively()
{
$basePath = $this->workspace.DIRECTORY_SEPARATOR.'directory'.DIRECTORY_SEPARATOR;
mkdir($basePath);
mkdir($basePath.'dir');
touch($basePath.'file');
$this->filesystem->remove($basePath);
$this->assertTrue(!is_dir($basePath));
}
public function testRemoveCleansArrayOfFilesAndDirectories()
{
$basePath = $this->workspace.DIRECTORY_SEPARATOR;
mkdir($basePath.'dir');
touch($basePath.'file');
$files = array(
$basePath.'dir', $basePath.'file'
);
$this->filesystem->remove($files);
$this->assertTrue(!is_dir($basePath.'dir'));
$this->assertTrue(!is_file($basePath.'file'));
}
public function testRemoveCleansTraversableObjectOfFilesAndDirectories()
{
$basePath = $this->workspace.DIRECTORY_SEPARATOR;
mkdir($basePath.'dir');
touch($basePath.'file');
$files = new \ArrayObject(array(
$basePath.'dir', $basePath.'file'
));
$this->filesystem->remove($files);
$this->assertTrue(!is_dir($basePath.'dir'));
$this->assertTrue(!is_file($basePath.'file'));
}
public function testRemoveIgnoresNonExistingFiles()
{
$basePath = $this->workspace.DIRECTORY_SEPARATOR;
mkdir($basePath.'dir');
$files = array(
$basePath.'dir', $basePath.'file'
);
$this->filesystem->remove($files);
$this->assertTrue(!is_dir($basePath.'dir'));
}
public function testRemoveCleansInvalidLinks()
{
$this->markAsSkippeIfSymlinkIsMissing();
$basePath = $this->workspace.DIRECTORY_SEPARATOR.'directory'.DIRECTORY_SEPARATOR;
mkdir($basePath);
mkdir($basePath.'dir');
// create symlink to unexisting file
symlink($basePath.'file', $basePath.'link');
$this->filesystem->remove($basePath);
$this->assertTrue(!is_dir($basePath));
}
public function testChmodChangesFileMode()
{
$file = $this->workspace.DIRECTORY_SEPARATOR.'file';
touch($file);
$this->filesystem->chmod($file, 0753);
$this->assertEquals(753, $this->getFilePermisions($file));
}
public function testChmodChangesModeOfArrayOfFiles()
{
$directory = $this->workspace.DIRECTORY_SEPARATOR.'directory';
$file = $this->workspace.DIRECTORY_SEPARATOR.'file';
$files = array($directory, $file);
mkdir($directory);
touch($file);
$this->filesystem->chmod($files, 0753);
$this->assertEquals(753, $this->getFilePermisions($file));
$this->assertEquals(753, $this->getFilePermisions($directory));
}
public function testChmodChangesModeOfTraversableFileObject()
{
$directory = $this->workspace.DIRECTORY_SEPARATOR.'directory';
$file = $this->workspace.DIRECTORY_SEPARATOR.'file';
$files = new \ArrayObject(array($directory, $file));
mkdir($directory);
touch($file);
$this->filesystem->chmod($files, 0753);
$this->assertEquals(753, $this->getFilePermisions($file));
$this->assertEquals(753, $this->getFilePermisions($directory));
}
public function testRename()
{
$file = $this->workspace.DIRECTORY_SEPARATOR.'file';
$newPath = $this->workspace.DIRECTORY_SEPARATOR.'new_file';
touch($file);
$this->filesystem->rename($file, $newPath);
$this->assertFileNotExists($file);
$this->assertFileExists($newPath);
}
/**
* @expectedException \RuntimeException
*/
public function testRenameThrowsExceptionIfTargetAlreadyExists()
{
$file = $this->workspace.DIRECTORY_SEPARATOR.'file';
$newPath = $this->workspace.DIRECTORY_SEPARATOR.'new_file';
touch($file);
touch($newPath);
$this->filesystem->rename($file, $newPath);
}
public function testSymlink()
{
$this->markAsSkippeIfSymlinkIsMissing();
$file = $this->workspace.DIRECTORY_SEPARATOR.'file';
$link = $this->workspace.DIRECTORY_SEPARATOR.'link';
touch($file);
$this->filesystem->symlink($file, $link);
$this->assertTrue(is_link($link));
$this->assertEquals($file, readlink($link));
}
public function testSymlinkIsOverwrittenIfPointsToDifferentTarget()
{
$this->markAsSkippeIfSymlinkIsMissing();
$file = $this->workspace.DIRECTORY_SEPARATOR.'file';
$link = $this->workspace.DIRECTORY_SEPARATOR.'link';
touch($file);
symlink($this->workspace, $link);
$this->filesystem->symlink($file, $link);
$this->assertTrue(is_link($link));
$this->assertEquals($file, readlink($link));
}
public function testSymlinkIsNotOverwrittenIfAlreadyCreated()
{
$this->markAsSkippeIfSymlinkIsMissing();
$file = $this->workspace.DIRECTORY_SEPARATOR.'file';
$link = $this->workspace.DIRECTORY_SEPARATOR.'link';
touch($file);
symlink($file, $link);
$this->filesystem->symlink($file, $link);
$this->assertTrue(is_link($link));
$this->assertEquals($file, readlink($link));
}
/**
* @dataProvider providePathsForMakePathRelative
*/
public function testMakePathRelative($endPath, $startPath, $expectedPath)
{
$path = $this->filesystem->makePathRelative($endPath, $startPath);
$this->assertEquals($expectedPath, $path);
}
/**
* @return array
*/
public function providePathsForMakePathRelative()
{
$paths = array(
array('/var/lib/symfony/src/Symfony/', '/var/lib/symfony/src/Symfony/Component', '../'),
array('var/lib/symfony/', 'var/lib/symfony/src/Symfony/Component', '../../../'),
array('/usr/lib/symfony/', '/var/lib/symfony/src/Symfony/Component', '../../../../../../usr/lib/symfony/'),
array('/var/lib/symfony/src/Symfony/', '/var/lib/symfony/', '../src/Symfony/'),
);
// fix directory separator
foreach ($paths as $i => $pathItems) {
foreach ($pathItems as $k => $path) {
$paths[$i][$k] = str_replace('/', DIRECTORY_SEPARATOR, $path);
}
}
return $paths;
}
public function testMirrorCopiesFilesAndDirectoriesRecursively()
{
$sourcePath = $this->workspace.DIRECTORY_SEPARATOR.'source'.DIRECTORY_SEPARATOR;
$directory = $sourcePath.'directory'.DIRECTORY_SEPARATOR;
$file1 = $directory.'file1';
$file2 = $sourcePath.'file2';
mkdir($sourcePath);
mkdir($directory);
file_put_contents($file1, 'FILE1');
file_put_contents($file2, 'FILE2');
$targetPath = $this->workspace.DIRECTORY_SEPARATOR.'target'.DIRECTORY_SEPARATOR;
$this->filesystem->mirror($sourcePath, $targetPath);
$this->assertTrue(is_dir($targetPath));
$this->assertTrue(is_dir($targetPath.'directory'));
$this->assertFileEquals($file1, $targetPath.'directory'.DIRECTORY_SEPARATOR.'file1');
$this->assertFileEquals($file2, $targetPath.'file2');
}
/**
* @dataProvider providePathsForIsAbsolutePath
*/
public function testIsAbsolutePath($path, $expectedResult)
{
$result = $this->filesystem->isAbsolutePath($path);
$this->assertEquals($expectedResult, $result);
}
/**
* @return array
*/
public function providePathsForIsAbsolutePath()
{
return array(
array('/var/lib', true),
array('c:\\\\var\\lib', true),
array('\\var\\lib', true),
array('var/lib', false),
array('../var/lib', false)
);
}
/**
* Returns file permissions as three digits (i.e. 755)
*
* @return integer
*/
private function getFilePermisions($filePath)
{
return (int) substr(sprintf('%o', fileperms($filePath)), -3);
}
private function markAsSkippeIfSymlinkIsMissing()
{
if (!function_exists('symlink')) {
$this->markTestSkipped('symlink is not supported');
}
}
}