From 31e8244562accb3087be6988240742d27e7d5bda Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Thu, 4 Feb 2021 17:01:27 +0300 Subject: [PATCH 01/11] Raise minimum stability --- composer.json | 2 -- 1 file changed, 2 deletions(-) diff --git a/composer.json b/composer.json index 7436bb0..39455de 100644 --- a/composer.json +++ b/composer.json @@ -42,8 +42,6 @@ "config": { "sort-packages": true }, - "minimum-stability": "dev", - "prefer-stable": true, "scripts": { "test": "phpunit --testdox --no-interaction", "test-watch": "phpunit-watcher watch" From 69d05be6fc9749e4646d94fcf83016b6f98ab98a Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Thu, 4 Feb 2021 17:15:56 +0300 Subject: [PATCH 02/11] phpdoc and minor code improvements --- README.md | 2 +- src/FileHelper.php | 76 +++++++++++++-------------- tests/FileHelperCopyDirectoryTest.php | 4 +- tests/FileHelperUnlinkTest.php | 10 ++-- tests/FileSystemTestCase.php | 3 +- tests/PathMatcher/PathPatternTest.php | 1 + 6 files changed, 49 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index ef01250..3e21939 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ $file = '/path/to/file.txt'; FileHelper::unlink($file); ``` -Normalize path: +Normalize a path: ```php use \Yiisoft\Files\FileHelper; diff --git a/src/FileHelper.php b/src/FileHelper.php index 58dbbab..3cd8a68 100644 --- a/src/FileHelper.php +++ b/src/FileHelper.php @@ -13,15 +13,15 @@ use RuntimeException; /** - * FileHelper provides useful methods to manage files and directories + * FileHelper provides useful methods to manage files and directories. */ class FileHelper { /** * Opens a file or URL. * - * This method is similar to the PHP `fopen()` function, except that it suppresses the `E_WARNING` - * level error and throws the `\RuntimeException` exception if it can't open the file. + * This method is similar to the PHP {@see fopen()} function, except that it suppresses the {@see E_WARNING} + * level error and throws the {@see \RuntimeException} exception if it can't open the file. * * @param string $filename The file or URL. * @param string $mode The type of access. @@ -48,13 +48,13 @@ public static function openFile(string $filename, string $mode, bool $useInclude /** * Creates a new directory. * - * This method is similar to the PHP `mkdir()` function except that it uses `chmod()` to set the permission of the + * This method is similar to the PHP {@see mkdir()} function except that it uses {@see chmod()} to set the permission of the * created directory in order to avoid the impact of the `umask` setting. * - * @param string $path path of the directory to be created. - * @param int $mode the permission to be set for the created directory. + * @param string $path Path of the directory to be created. + * @param int $mode The permission to be set for the created directory. * - * @return bool whether the directory is created successfully. + * @return bool Whether the directory is created successfully. */ public static function createDirectory(string $path, int $mode = 0775): bool { @@ -87,9 +87,9 @@ public static function createDirectory(string $path, int $mode = 0775): bool * - Turn multiple consecutive slashes into a single one (e.g. "/a///b/c" becomes "/a/b/c") * - Remove ".." and "." based on their meanings (e.g. "/a/./b/../c" becomes "/a/c") * - * @param string $path the file/directory path to be normalized + * @param string $path The file/directory path to be normalized. * - * @return string the normalized file/directory path + * @return string The normalized file/directory path. */ public static function normalizePath(string $path): string { @@ -127,8 +127,8 @@ public static function normalizePath(string $path): string /** * Removes a directory (and all its content) recursively. * - * @param string $directory the directory to be deleted recursively. - * @param array $options options for directory remove ({@see clearDirectory()}). + * @param string $directory The directory to be deleted recursively. + * @param array $options Options for directory remove ({@see clearDirectory()}). */ public static function removeDirectory(string $directory, array $options = []): void { @@ -148,14 +148,14 @@ public static function removeDirectory(string $directory, array $options = []): /** * Clear all directory content. * - * @param string $directory the directory to be cleared. - * @param array $options options for directory clear . Valid options are: + * @param string $directory The directory to be cleared. + * @param array $options Options for directory clear . Valid options are: * * - traverseSymlinks: boolean, whether symlinks to the directories should be traversed too. * Defaults to `false`, meaning the content of the symlinked directory would not be deleted. * Only symlink would be removed in that default case. * - * @throws InvalidArgumentException if unable to open directory + * @throws InvalidArgumentException if unable to open directory. */ public static function clearDirectory(string $directory, array $options = []): void { @@ -179,7 +179,7 @@ public static function clearDirectory(string $directory, array $options = []): v /** * Removes a file or symlink in a cross-platform way. * - * @param string $path + * @param string $path Path to unlink. */ public static function unlink(string $path): void { @@ -204,9 +204,9 @@ public static function unlink(string $path): void } /** - * Tells whether the path is a empty directory + * Tells whether the path is a empty directory. * - * @param string $path + * @param string $path Path to check for being an empty directory. * * @return bool */ @@ -224,9 +224,9 @@ public static function isEmptyDirectory(string $path): bool * * The files and sub-directories will also be copied over. * - * @param string $source the source directory. - * @param string $destination the destination directory. - * @param array $options options for directory copy. Valid options are: + * @param string $source The source directory. + * @param string $destination The destination directory. + * @param array $options Options for directory copy. Valid options are: * * - dirMode: integer, the permission to be set for newly copied directories. Defaults to 0775. * - fileMode: integer, the permission to be set for newly copied files. Defaults to the current environment @@ -308,10 +308,10 @@ public static function copyDirectory(string $source, string $destination, array } /** - * Check copy it self directory. + * Assert that destination is not within the source directory. * - * @param string $source - * @param string $destination + * @param string $source Path to source. + * @param string $destination Path to destination. * * @throws InvalidArgumentException */ @@ -325,7 +325,7 @@ private static function assertNotSelfDirectory(string $source, string $destinati /** * Open directory handle. * - * @param string $directory + * @param string $directory Path to directory. * * @throws InvalidArgumentException * @@ -336,22 +336,22 @@ private static function openDirectory(string $directory) $handle = @opendir($directory); if ($handle === false) { - throw new InvalidArgumentException("Unable to open directory: $directory"); + throw new InvalidArgumentException("Unable to open directory: $directory."); } return $handle; } /** - * Returns the last modification time for the given path. + * Returns the last modification time for the given paths. * * If the path is a directory, any nested files/directories will be checked as well. * - * @param string ...$paths the directory to be checked + * @param string ...$paths The directories to be checked. * - * @throws LogicException when path not set + * @throws LogicException If path is not set. * - * @return int Unix timestamp representing the last modification time + * @return int Unix timestamp representing the last modification time. */ public static function lastModifiedTime(string ...$paths): int { @@ -391,8 +391,8 @@ private static function modifiedTime(string $path): int /** * Returns the directories found under the specified directory and subdirectories. * - * @param string $directory the directory under which the files will be looked for. - * @param array $options options for directory searching. Valid options are: + * @param string $directory The directory under which the files will be looked for. + * @param array $options Options for directory searching. Valid options are: * * - filter: a filter to apply while looked directories. It should be an instance of {@see PathMatcherInterface}. * - recursive: boolean, whether the subdirectories should also be looked for. Defaults to `true`. @@ -402,10 +402,10 @@ private static function modifiedTime(string $path): int * recursive?: bool, * } $options * - * @throws InvalidArgumentException if the directory is invalid. + * @throws InvalidArgumentException If the directory is invalid. * - * @return string[] directories found under the directory specified, in no particular order. - * Ordering depends on the files system used. + * @return string[] Directories found under the directory specified, in no particular order. + * Ordering depends on the file system used. */ public static function findDirectories(string $directory, array $options = []): array { @@ -443,8 +443,8 @@ public static function findDirectories(string $directory, array $options = []): /** * Returns the files found under the specified directory and subdirectories. * - * @param string $directory the directory under which the files will be looked for. - * @param array $options options for file searching. Valid options are: + * @param string $directory The directory under which the files will be looked for. + * @param array $options Options for file searching. Valid options are: * * - filter: a filter to apply while looked files. It should be an instance of {@see PathMatcherInterface}. * - recursive: boolean, whether the files under the subdirectories should also be looked for. Defaults to `true`. @@ -454,9 +454,9 @@ public static function findDirectories(string $directory, array $options = []): * recursive?: bool, * } $options * - * @throws InvalidArgumentException if the dir is invalid. + * @throws InvalidArgumentException If the directory is invalid. * - * @return array files found under the directory specified, in no particular order. + * @return array Files found under the directory specified, in no particular order. * Ordering depends on the files system used. */ public static function findFiles(string $directory, array $options = []): array diff --git a/tests/FileHelperCopyDirectoryTest.php b/tests/FileHelperCopyDirectoryTest.php index bd4dbd6..816bc03 100644 --- a/tests/FileHelperCopyDirectoryTest.php +++ b/tests/FileHelperCopyDirectoryTest.php @@ -144,7 +144,7 @@ public function testPermissions(): void } /** - * Copy directory to it self. + * Copy directory to itself. * * @see https://github.com/yiisoft/yii2/issues/10710 */ @@ -163,7 +163,7 @@ public function testCopyToItself(): void } /** - * Copy directory to subdirectory of it self. + * Copy directory to subdirectory of itself. * * @see https://github.com/yiisoft/yii2/issues/10710 */ diff --git a/tests/FileHelperUnlinkTest.php b/tests/FileHelperUnlinkTest.php index 9be8333..6331864 100644 --- a/tests/FileHelperUnlinkTest.php +++ b/tests/FileHelperUnlinkTest.php @@ -28,9 +28,9 @@ public function testUnlinkSymlink(): void $this->assertFileDoesNotExist($symlinkedFilePath); } - public function testUnlinkSymlinkToNonexistentsDirectory(): void + public function testUnlinkSymlinkToNonExistingDirectory(): void { - $dirName = 'unlink-symlink-to-nonexistents-directory'; + $dirName = 'unlink-symlink-to-non-existing-directory'; $this->createFileStructure([ $dirName => [ 'dir' => [ @@ -50,9 +50,9 @@ public function testUnlinkSymlinkToNonexistentsDirectory(): void $this->assertFalse(is_link($basePath . '/symlink')); } - public function testUnlinkSymlinkToNonexistentsFile(): void + public function testUnlinkSymlinkToNonExistingFile(): void { - $dirName = 'unlink-symlink-to-nonexistents-file'; + $dirName = 'unlink-symlink-to-non-existing-file'; $this->createFileStructure([ $dirName => [ 'file.txt' => 'content', @@ -70,7 +70,7 @@ public function testUnlinkSymlinkToNonexistentsFile(): void } /** - * 777 gives "read only" flag under Windows + * 777 gives "read only" flag under Windows. * * @see https://github.com/yiisoft/files/issues/21 */ diff --git a/tests/FileSystemTestCase.php b/tests/FileSystemTestCase.php index 2103550..72e272b 100644 --- a/tests/FileSystemTestCase.php +++ b/tests/FileSystemTestCase.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\TestCase; use Yiisoft\Files\FileHelper; +use function is_array; abstract class FileSystemTestCase extends TestCase { @@ -34,7 +35,7 @@ public function tearDown(): void * Creates test files structure. * * @param array $items file system objects to be created in format: objectName => objectContent - * Arrays specifies directories, other values - files. + * Arrays specifies directories, other values - files. * @param string|null $basePath structure base file path. */ protected function createFileStructure(array $items, ?string $basePath = null): void diff --git a/tests/PathMatcher/PathPatternTest.php b/tests/PathMatcher/PathPatternTest.php index 5fd615d..c6299d1 100644 --- a/tests/PathMatcher/PathPatternTest.php +++ b/tests/PathMatcher/PathPatternTest.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\TestCase; use Yiisoft\Files\PathMatcher\PathPattern; +use function in_array; final class PathPatternTest extends TestCase { From 46e3f284bef76fa884afbdf2978a189cc946184a Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Thu, 4 Feb 2021 18:23:29 +0300 Subject: [PATCH 03/11] Fix readme --- README.md | 105 ++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 78 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 3e21939..9a69810 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,11 @@ The package could be installed with composer: composer require yiisoft/files --prefer-dist ``` -## General usage +## FileHelper usage + +FileHelper provides static methods you can use for various filesystem-related operations. + +### Working with directories Create a new directory: @@ -58,55 +62,102 @@ $directory = '/path/to/dir'; FileHelper::removeDirectory($directory); ``` -Remove a file or symlink: +Remove everything within a directory but not directory itself: ```php use \Yiisoft\Files\FileHelper; -$file = '/path/to/file.txt'; -FileHelper::unlink($file); +$directory = '/path/to/dir'; +FileHelper::clearDirectory($directory); ``` -Normalize a path: +Check if directory is empty: ```php use \Yiisoft\Files\FileHelper; -$path = '/home/samdark/./test/..///dev\yii/'; -echo FileHelper::normalizePath($path); -// outputs: -// /home/samdark/dev/yii +$directory = '/path/to/dir'; +FileHelper::isEmptyDirectory($directory); ``` -## File helper usage - -File helper methods are static so usage is like the following: +Copy directory: ```php -\Yiisoft\Files\FileHelper::copyDirectory('/home/master/inbox', '/backup/inbox') ?> +use \Yiisoft\Files\FileHelper; + +$source = '/path/to/source'; +$destination = '/path/to/destination'; +FileHelper::copyDirectory($source, $destination); ``` -Overall the helper has the following method groups. +Additional options could be specified as third argument such as `filter` or `copyEmptyDirectories`. +Check method phpdoc for a full list of options. -### Working with directories +### Search -- createDirectory -- copyDirectory -- clearDirectory -- removeDirectory -- isEmptyDirectory +Searching for files: -### Search +```php +use \Yiisoft\Files\FileHelper; +use Yiisoft\Files\PathMatcher\PathMatcher; -- findDirectories -- findFiles +$files = FileHelper::findFiles('/path/to/where/to/search', [ + 'filter' => (new PathMatcher())->only('*.png', '*.jpg')->except('logo.png'), +]); +``` + +Searching for directories: + +```php +use \Yiisoft\Files\FileHelper; +use Yiisoft\Files\PathMatcher\PathMatcher; + +$directories = FileHelper::findDirectories('/path/to/where/to/search', [ + 'filter' => (new PathMatcher())->except('*meta'), +]); +``` ### Other -- openFile -- normalizePath -- unlink -- lastModifiedTime +Open a file. Same as PHP's `fopen()` but throwing exceptions. + +```php +use \Yiisoft\Files\FileHelper; + +$handler = FileHelper::openFile('/path/to/file', 'rb'); +``` + +Get last modified time for a directory or file: + +```php +use \Yiisoft\Files\FileHelper; + +$directory = '/path/to/dir'; +$time = FileHelper::lastModifiedTime($directory); +``` + +The method is different from PHP's `filemtime()` because it actually scans a directory and selects the largest +modification time from all files. + +Remove a file or symlink: + +```php +use \Yiisoft\Files\FileHelper; + +$file = '/path/to/file.txt'; +FileHelper::unlink($file); +``` + +Normalize a path: + +```php +use \Yiisoft\Files\FileHelper; + +$path = '/home/samdark/./test/..///dev\yii/'; +echo FileHelper::normalizePath($path); +// outputs: +// /home/samdark/dev/yii +``` ## Testing From bcd0498aec833294647f933d336352eed2580de4 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Thu, 4 Feb 2021 18:24:04 +0300 Subject: [PATCH 04/11] Improve code, add extra checks to find* methods --- src/FileHelper.php | 16 ++++++++++++++++ src/PathMatcher/PathMatcher.php | 4 ++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/FileHelper.php b/src/FileHelper.php index 3cd8a68..d27c717 100644 --- a/src/FileHelper.php +++ b/src/FileHelper.php @@ -11,6 +11,10 @@ use RecursiveDirectoryIterator; use RecursiveIteratorIterator; use RuntimeException; +use Yiisoft\Files\PathMatcher\PathMatcherInterface; +use function array_key_exists; +use function get_class; +use function is_object; /** * FileHelper provides useful methods to manage files and directories. @@ -412,6 +416,12 @@ public static function findDirectories(string $directory, array $options = []): if (!is_dir($directory)) { throw new InvalidArgumentException("\"$directory\" is not a directory."); } + + if (array_key_exists('filter', $options) && !$options['filter'] instanceof PathMatcherInterface) { + $type = is_object($options['filter']) ? get_class($options['filter']) : gettype($options['filter']); + throw new InvalidArgumentException(sprintf('Filter should be an instance of PathMatcherInterface, %s given.', $type)); + } + $directory = static::normalizePath($directory); $result = []; @@ -464,6 +474,12 @@ public static function findFiles(string $directory, array $options = []): array if (!is_dir($directory)) { throw new InvalidArgumentException("\"$directory\" is not a directory."); } + + if (array_key_exists('filter', $options) && !$options['filter'] instanceof PathMatcherInterface) { + $type = is_object($options['filter']) ? get_class($options['filter']) : gettype($options['filter']); + throw new InvalidArgumentException(sprintf('Filter should be an instance of PathMatcherInterface, %s given.', $type)); + } + $directory = static::normalizePath($directory); $result = []; diff --git a/src/PathMatcher/PathMatcher.php b/src/PathMatcher/PathMatcher.php index d36a0ae..3b5e906 100644 --- a/src/PathMatcher/PathMatcher.php +++ b/src/PathMatcher/PathMatcher.php @@ -209,10 +209,10 @@ private function matchOnly(string $path): bool if ($this->checkFilesystem) { if (is_file($path)) { - return $hasFalse ? false : true; + return !$hasFalse; } if (is_dir($path)) { - return $hasNull ? true : false; + return $hasNull; } } From d569fcc4ba477dec1d3860ccba0aa3cfceb1208d Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Thu, 4 Feb 2021 15:24:31 +0000 Subject: [PATCH 05/11] Apply fixes from StyleCI --- src/FileHelper.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/FileHelper.php b/src/FileHelper.php index d27c717..ac04b3c 100644 --- a/src/FileHelper.php +++ b/src/FileHelper.php @@ -418,7 +418,7 @@ public static function findDirectories(string $directory, array $options = []): } if (array_key_exists('filter', $options) && !$options['filter'] instanceof PathMatcherInterface) { - $type = is_object($options['filter']) ? get_class($options['filter']) : gettype($options['filter']); + $type = is_object($options['filter']) ? get_class($options['filter']) : gettype($options['filter']); throw new InvalidArgumentException(sprintf('Filter should be an instance of PathMatcherInterface, %s given.', $type)); } @@ -476,7 +476,7 @@ public static function findFiles(string $directory, array $options = []): array } if (array_key_exists('filter', $options) && !$options['filter'] instanceof PathMatcherInterface) { - $type = is_object($options['filter']) ? get_class($options['filter']) : gettype($options['filter']); + $type = is_object($options['filter']) ? get_class($options['filter']) : gettype($options['filter']); throw new InvalidArgumentException(sprintf('Filter should be an instance of PathMatcherInterface, %s given.', $type)); } From 9417dfd98535962f109703ae40b5d75159ca36a5 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Thu, 4 Feb 2021 20:42:17 +0300 Subject: [PATCH 06/11] Fix createDirectory() --- README.md | 4 ++-- src/FileHelper.php | 35 +++++++++++++++++++++-------------- tests/FileHelperTest.php | 4 ++-- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 9a69810..ee42a2d 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ FileHelper provides static methods you can use for various filesystem-related op ### Working with directories -Create a new directory: +Create a directory: ```php use \Yiisoft\Files\FileHelper; @@ -44,7 +44,7 @@ $directory = '/path/to/dir'; FileHelper::createDirectory($directory); ``` -Create a new directory with the permission to be set: +Create a directory, and set permission specified: ```php use \Yiisoft\Files\FileHelper; diff --git a/src/FileHelper.php b/src/FileHelper.php index ac04b3c..50b7289 100644 --- a/src/FileHelper.php +++ b/src/FileHelper.php @@ -52,33 +52,40 @@ public static function openFile(string $filename, string $mode, bool $useInclude /** * Creates a new directory. * - * This method is similar to the PHP {@see mkdir()} function except that it uses {@see chmod()} to set the permission of the - * created directory in order to avoid the impact of the `umask` setting. + * This method is similar to the PHP {@see mkdir()} function with some differences: + * + * - It does not fail if directory already exists. + * - It uses {@see chmod()} to set the permission of the created directory in order to avoid the impact + * of the `umask` setting. + * - It throws exceptions instead of returning false and emitting {@see E_WARNING}. * * @param string $path Path of the directory to be created. * @param int $mode The permission to be set for the created directory. - * - * @return bool Whether the directory is created successfully. */ - public static function createDirectory(string $path, int $mode = 0775): bool + public static function createDirectory(string $path, int $mode = 0775): void { $path = static::normalizePath($path); - try { + if (!is_dir($path)) { + set_error_handler(static function (int $errorNumber, string $errorString) use ($path) { + throw new RuntimeException( + sprintf('Failed to create directory "%s". ', $path) . $errorString, + $errorNumber, + null + ); + }); + + // See https://github.com/kalessil/phpinspectionsea/blob/master/docs/probable-bugs.md#mkdir-race-condition if (!mkdir($path, $mode, true) && !is_dir($path)) { - return false; - } - } catch (Exception $e) { - if (!is_dir($path)) { throw new RuntimeException( - 'Failed to create directory "' . $path . '": ' . $e->getMessage(), - (int)$e->getCode(), - $e + sprintf('Failed to create directory "%s".', $path), ); } + + restore_error_handler(); } - return chmod($path, $mode); + chmod($path, $mode); } /** diff --git a/tests/FileHelperTest.php b/tests/FileHelperTest.php index 7c3a966..ba01451 100644 --- a/tests/FileHelperTest.php +++ b/tests/FileHelperTest.php @@ -37,9 +37,9 @@ public function testCreateDirectory(): void { $basePath = $this->testFilePath; $directory = $basePath . '/test_dir_level_1/test_dir_level_2'; - $this->assertTrue(FileHelper::createDirectory($directory), 'FileHelper::createDirectory should return true if directory was created!'); + FileHelper::createDirectory($directory); $this->assertFileExists($directory, 'Unable to create directory recursively!'); - $this->assertTrue(FileHelper::createDirectory($directory), 'FileHelper::createDirectory should return true for already existing directories!'); + FileHelper::createDirectory($directory); } public function testCreateDirectoryPermissions(): void From ed4fb6959afa5835bd31e464999972bbab5a21fb Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Thu, 4 Feb 2021 20:44:30 +0300 Subject: [PATCH 07/11] Rename createDirectory() -> ensureDirectory() --- README.md | 8 ++++---- src/FileHelper.php | 8 ++++---- tests/FileHelperTest.php | 8 ++++---- tests/FileSystemTestCase.php | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index ee42a2d..21cc4c8 100644 --- a/README.md +++ b/README.md @@ -35,22 +35,22 @@ FileHelper provides static methods you can use for various filesystem-related op ### Working with directories -Create a directory: +Ensure a directory exists: ```php use \Yiisoft\Files\FileHelper; $directory = '/path/to/dir'; -FileHelper::createDirectory($directory); +FileHelper::ensureDirectory($directory); ``` -Create a directory, and set permission specified: +Ensure a directory exists, and permission specified is set: ```php use \Yiisoft\Files\FileHelper; $directory = '/path/to/dir'; -FileHelper::createDirectory($directory, 0775); +FileHelper::ensureDirectory($directory, 0775); ``` Remove a directory: diff --git a/src/FileHelper.php b/src/FileHelper.php index 50b7289..e1e0374 100644 --- a/src/FileHelper.php +++ b/src/FileHelper.php @@ -50,7 +50,7 @@ public static function openFile(string $filename, string $mode, bool $useInclude } /** - * Creates a new directory. + * Ensures directory exists and has specific permissions. * * This method is similar to the PHP {@see mkdir()} function with some differences: * @@ -62,7 +62,7 @@ public static function openFile(string $filename, string $mode, bool $useInclude * @param string $path Path of the directory to be created. * @param int $mode The permission to be set for the created directory. */ - public static function createDirectory(string $path, int $mode = 0775): void + public static function ensureDirectory(string $path, int $mode = 0775): void { $path = static::normalizePath($path); @@ -281,7 +281,7 @@ public static function copyDirectory(string $source, string $destination, array !$destinationExists && (!isset($options['copyEmptyDirectories']) || $options['copyEmptyDirectories']) ) { - static::createDirectory($destination, $options['dirMode'] ?? 0775); + static::ensureDirectory($destination, $options['dirMode'] ?? 0775); $destinationExists = true; } @@ -302,7 +302,7 @@ public static function copyDirectory(string $source, string $destination, array if (!isset($options['filter']) || $options['filter']->match($from)) { if (is_file($from)) { if (!$destinationExists) { - static::createDirectory($destination, $options['dirMode'] ?? 0775); + static::ensureDirectory($destination, $options['dirMode'] ?? 0775); $destinationExists = true; } copy($from, $to); diff --git a/tests/FileHelperTest.php b/tests/FileHelperTest.php index ba01451..3dacc00 100644 --- a/tests/FileHelperTest.php +++ b/tests/FileHelperTest.php @@ -37,9 +37,9 @@ public function testCreateDirectory(): void { $basePath = $this->testFilePath; $directory = $basePath . '/test_dir_level_1/test_dir_level_2'; - FileHelper::createDirectory($directory); + FileHelper::ensureDirectory($directory); $this->assertFileExists($directory, 'Unable to create directory recursively!'); - FileHelper::createDirectory($directory); + FileHelper::ensureDirectory($directory); } public function testCreateDirectoryPermissions(): void @@ -51,7 +51,7 @@ public function testCreateDirectoryPermissions(): void $basePath = $this->testFilePath; $dirName = $basePath . '/test_dir_perms'; - $this->assertTrue(FileHelper::createDirectory($dirName, 0700)); + $this->assertTrue(FileHelper::ensureDirectory($dirName, 0700)); $this->assertFileMode(0700, $dirName); } @@ -65,7 +65,7 @@ public function testCreateDirectoryException(): void $this->expectException(RuntimeException::class); $this->expectExceptionMessageMatches('/^Failed to create directory/'); - FileHelper::createDirectory($this->testFilePath . '/test_dir/file.txt'); + FileHelper::ensureDirectory($this->testFilePath . '/test_dir/file.txt'); } public function testRemoveDirectory(): void diff --git a/tests/FileSystemTestCase.php b/tests/FileSystemTestCase.php index 72e272b..9f0a834 100644 --- a/tests/FileSystemTestCase.php +++ b/tests/FileSystemTestCase.php @@ -19,7 +19,7 @@ public function setUp(): void { $this->testFilePath = FileHelper::normalizePath(realpath(sys_get_temp_dir()) . '/' . static::class); - FileHelper::createDirectory($this->testFilePath, 0777); + FileHelper::ensureDirectory($this->testFilePath, 0777); if (!file_exists($this->testFilePath)) { $this->markTestIncomplete('Unit tests runtime directory should have writable permissions.'); From 627ecc5dacffef7530c1b8c26bab4398e2cfb8ce Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Thu, 4 Feb 2021 21:34:49 +0300 Subject: [PATCH 08/11] Better exception and concurrency handling --- src/FileHelper.php | 90 +++++++++++++++++++-------- tests/FileHelperCopyDirectoryTest.php | 2 +- tests/FileHelperTest.php | 2 +- tests/FileHelperUnlinkTest.php | 3 +- 4 files changed, 68 insertions(+), 29 deletions(-) diff --git a/src/FileHelper.php b/src/FileHelper.php index e1e0374..e0bf76a 100644 --- a/src/FileHelper.php +++ b/src/FileHelper.php @@ -4,7 +4,6 @@ namespace Yiisoft\Files; -use Exception; use FilesystemIterator; use InvalidArgumentException; use LogicException; @@ -40,10 +39,19 @@ class FileHelper */ public static function openFile(string $filename, string $mode, bool $useIncludePath = false, $context = null) { - $filePointer = @fopen($filename, $mode, $useIncludePath, $context); + set_error_handler(static function (int $errorNumber, string $errorString) use ($filename) { + throw new RuntimeException( + sprintf('Failed to open a file "%s". ', $filename) . $errorString, + $errorNumber + ); + }); + + $filePointer = fopen($filename, $mode, $useIncludePath, $context); + + restore_error_handler(); if ($filePointer === false) { - throw new RuntimeException("The file \"{$filename}\" could not be opened."); + throw new RuntimeException(sprintf('Failed to open a file "%s". ', $filename)); } return $filePointer; @@ -68,24 +76,24 @@ public static function ensureDirectory(string $path, int $mode = 0775): void if (!is_dir($path)) { set_error_handler(static function (int $errorNumber, string $errorString) use ($path) { - throw new RuntimeException( - sprintf('Failed to create directory "%s". ', $path) . $errorString, - $errorNumber, - null - ); + // Handle race condition. + // See https://github.com/kalessil/phpinspectionsea/blob/master/docs/probable-bugs.md#mkdir-race-condition + if (!is_dir($path)) { + throw new RuntimeException( + sprintf('Failed to create directory "%s". ', $path) . $errorString, + $errorNumber + ); + } }); - // See https://github.com/kalessil/phpinspectionsea/blob/master/docs/probable-bugs.md#mkdir-race-condition - if (!mkdir($path, $mode, true) && !is_dir($path)) { - throw new RuntimeException( - sprintf('Failed to create directory "%s".', $path), - ); - } + mkdir($path, $mode, true); restore_error_handler(); } - chmod($path, $mode); + if (!chmod($path, $mode)) { + throw new RuntimeException(sprintf('Unable to set mode "%s" for "%s".', $mode, $path)); + } } /** @@ -136,23 +144,34 @@ public static function normalizePath(string $path): string } /** - * Removes a directory (and all its content) recursively. + * Removes a directory (and all its content) recursively. Does nothing if directory does not exists. * * @param string $directory The directory to be deleted recursively. * @param array $options Options for directory remove ({@see clearDirectory()}). + * + * @throw RuntimeException when unable to remove directory. */ public static function removeDirectory(string $directory, array $options = []): void { - try { - static::clearDirectory($directory, $options); - } catch (InvalidArgumentException $e) { + if (!file_exists($directory)) { return; } + static::clearDirectory($directory, $options); + if (is_link($directory)) { self::unlink($directory); } else { + set_error_handler(static function (int $errorNumber, string $errorString) use ($directory) { + throw new RuntimeException( + sprintf('Failed to remove directory "%s". ', $directory) . $errorString, + $errorNumber + ); + }); + rmdir($directory); + + restore_error_handler(); } } @@ -166,7 +185,7 @@ public static function removeDirectory(string $directory, array $options = []): * Defaults to `false`, meaning the content of the symlinked directory would not be deleted. * Only symlink would be removed in that default case. * - * @throws InvalidArgumentException if unable to open directory. + * @throws RuntimeException if unable to open directory. */ public static function clearDirectory(string $directory, array $options = []): void { @@ -194,6 +213,13 @@ public static function clearDirectory(string $directory, array $options = []): v */ public static function unlink(string $path): void { + set_error_handler(static function (int $errorNumber, string $errorString) use ($path) { + throw new RuntimeException( + sprintf('Failed to unlink "%s". ', $path) . $errorString, + $errorNumber + ); + }); + $isWindows = DIRECTORY_SEPARATOR === '\\'; if (!$isWindows) { @@ -202,7 +228,9 @@ public static function unlink(string $path): void } if (is_link($path)) { - if (false === @unlink($path)) { + try { + unlink($path); + } catch (RuntimeException $e) { rmdir($path); } return; @@ -212,6 +240,8 @@ public static function unlink(string $path): void chmod($path, 0777); } unlink($path); + + restore_error_handler(); } /** @@ -256,8 +286,7 @@ public static function isEmptyDirectory(string $path): bool * directories that do not contain files at the target destination because files have been filtered via `only` or * `except`. Defaults to true. * - * @throws InvalidArgumentException if unable to open directory - * @throws Exception + * @throws RuntimeException if unable to open directory * * @psalm-param array{ * dirMode?: int, @@ -338,18 +367,27 @@ private static function assertNotSelfDirectory(string $source, string $destinati * * @param string $directory Path to directory. * - * @throws InvalidArgumentException + * @throws RuntimeException * * @return resource */ private static function openDirectory(string $directory) { - $handle = @opendir($directory); + set_error_handler(static function (int $errorNumber, string $errorString) use ($directory) { + throw new RuntimeException( + sprintf('Unable to open directory "%s". ', $directory) . $errorString, + $errorNumber + ); + }); + + $handle = opendir($directory); if ($handle === false) { - throw new InvalidArgumentException("Unable to open directory: $directory."); + throw new RuntimeException(sprintf('Unable to open directory "%s". ', $directory)); } + restore_error_handler(); + return $handle; } diff --git a/tests/FileHelperCopyDirectoryTest.php b/tests/FileHelperCopyDirectoryTest.php index 816bc03..ed265f1 100644 --- a/tests/FileHelperCopyDirectoryTest.php +++ b/tests/FileHelperCopyDirectoryTest.php @@ -224,7 +224,7 @@ public function testCopyWithSameName(): void public function testCopyNotExistsDirectory(): void { $dir = $this->testFilePath . '/not_exists_dir'; - $this->expectExceptionMessage('Unable to open directory: ' . $dir); + $this->expectExceptionMessage(sprintf('Unable to open directory "%s".', $dir)); FileHelper::copyDirectory($dir, $this->testFilePath . '/copy'); } diff --git a/tests/FileHelperTest.php b/tests/FileHelperTest.php index 3dacc00..9903479 100644 --- a/tests/FileHelperTest.php +++ b/tests/FileHelperTest.php @@ -198,7 +198,7 @@ public function testClearDirectory(): void $this->assertFileDoesNotExist($dirName . '/file1.txt'); $this->assertDirectoryDoesNotExist($dirName . '/test_sub_dir'); - $this->expectException(\InvalidArgumentException::class); + $this->expectException(RuntimeException::class); FileHelper::clearDirectory($this->testFilePath . '/nonExisting'); } diff --git a/tests/FileHelperUnlinkTest.php b/tests/FileHelperUnlinkTest.php index 6331864..e9f29c9 100644 --- a/tests/FileHelperUnlinkTest.php +++ b/tests/FileHelperUnlinkTest.php @@ -4,6 +4,7 @@ namespace Yiisoft\Files\Tests; +use RuntimeException; use Yiisoft\Files\FileHelper; final class FileHelperUnlinkTest extends FileSystemTestCase @@ -117,7 +118,7 @@ public function testDirectory(): void public function testUnlinkNonexistentFile(): void { - $this->expectWarning(); + $this->expectException(RuntimeException::class); FileHelper::unlink($this->testFilePath . '/not-exists-file.txt'); } } From 5f0cb7c62c55a537a62b6750b13ee4099c1ed6dd Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Thu, 4 Feb 2021 22:15:37 +0300 Subject: [PATCH 09/11] Optimize code --- src/FileHelper.php | 69 +++++++++++++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 22 deletions(-) diff --git a/src/FileHelper.php b/src/FileHelper.php index e0bf76a..c37f93e 100644 --- a/src/FileHelper.php +++ b/src/FileHelper.php @@ -39,7 +39,7 @@ class FileHelper */ public static function openFile(string $filename, string $mode, bool $useIncludePath = false, $context = null) { - set_error_handler(static function (int $errorNumber, string $errorString) use ($filename) { + set_error_handler(static function (int $errorNumber, string $errorString) use ($filename): ?bool { throw new RuntimeException( sprintf('Failed to open a file "%s". ', $filename) . $errorString, $errorNumber @@ -162,7 +162,7 @@ public static function removeDirectory(string $directory, array $options = []): if (is_link($directory)) { self::unlink($directory); } else { - set_error_handler(static function (int $errorNumber, string $errorString) use ($directory) { + set_error_handler(static function (int $errorNumber, string $errorString, string $errorFile, int $errorLine, array $context) use ($directory) { throw new RuntimeException( sprintf('Failed to remove directory "%s". ', $directory) . $errorString, $errorNumber @@ -300,6 +300,19 @@ public static function isEmptyDirectory(string $path): bool */ public static function copyDirectory(string $source, string $destination, array $options = []): void { + $filter = null; + if (array_key_exists('filter', $options)) { + if (!$options['filter'] instanceof PathMatcherInterface) { + $type = is_object($options['filter']) ? get_class($options['filter']) : gettype($options['filter']); + throw new InvalidArgumentException(sprintf('Filter should be an instance of PathMatcherInterface, %s given.', $type)); + } + $filter = $options['filter']; + } + + $recursive = !array_key_exists('recursive', $options) || $options['recursive']; + $fileMode = $options['fileMode'] ?? null; + $dirMode = $options['dirMode'] ?? 0775; + $source = static::normalizePath($source); $destination = static::normalizePath($destination); @@ -308,15 +321,15 @@ public static function copyDirectory(string $source, string $destination, array $destinationExists = is_dir($destination); if ( !$destinationExists && - (!isset($options['copyEmptyDirectories']) || $options['copyEmptyDirectories']) + (!array_key_exists('copyEmptyDirectories', $options) || $options['copyEmptyDirectories']) ) { - static::ensureDirectory($destination, $options['dirMode'] ?? 0775); + static::ensureDirectory($destination, $dirMode); $destinationExists = true; } $handle = static::openDirectory($source); - if (!isset($options['basePath'])) { + if (!array_key_exists('basePath', $options)) { $options['basePath'] = realpath($source); } @@ -328,17 +341,17 @@ public static function copyDirectory(string $source, string $destination, array $from = $source . '/' . $file; $to = $destination . '/' . $file; - if (!isset($options['filter']) || $options['filter']->match($from)) { + if ($filter === null || $filter->match($from)) { if (is_file($from)) { if (!$destinationExists) { - static::ensureDirectory($destination, $options['dirMode'] ?? 0775); + static::ensureDirectory($destination, $dirMode); $destinationExists = true; } copy($from, $to); - if (isset($options['fileMode'])) { - chmod($to, $options['fileMode']); + if ($fileMode !== null) { + chmod($to, $fileMode); } - } elseif (!isset($options['recursive']) || $options['recursive']) { + } elseif ($recursive) { static::copyDirectory($from, $to, $options); } } @@ -447,7 +460,7 @@ private static function modifiedTime(string $path): int * - recursive: boolean, whether the subdirectories should also be looked for. Defaults to `true`. * * @psalm-param array{ - * filter?: \Yiisoft\Files\PathMatcher\PathMatcherInterface, + * filter?: \Yiisoft\Files\PathMatcher\PathMatcherInterface|mixed, * recursive?: bool, * } $options * @@ -462,11 +475,17 @@ public static function findDirectories(string $directory, array $options = []): throw new InvalidArgumentException("\"$directory\" is not a directory."); } - if (array_key_exists('filter', $options) && !$options['filter'] instanceof PathMatcherInterface) { - $type = is_object($options['filter']) ? get_class($options['filter']) : gettype($options['filter']); - throw new InvalidArgumentException(sprintf('Filter should be an instance of PathMatcherInterface, %s given.', $type)); + $filter = null; + if (array_key_exists('filter', $options)) { + if (!$options['filter'] instanceof PathMatcherInterface) { + $type = is_object($options['filter']) ? get_class($options['filter']) : gettype($options['filter']); + throw new InvalidArgumentException(sprintf('Filter should be an instance of PathMatcherInterface, %s given.', $type)); + } + $filter = $options['filter']; } + $recursive = !array_key_exists('recursive', $options) || $options['recursive']; + $directory = static::normalizePath($directory); $result = []; @@ -482,11 +501,11 @@ public static function findDirectories(string $directory, array $options = []): continue; } - if (!isset($options['filter']) || $options['filter']->match($path)) { + if ($filter === null || $filter->match($path)) { $result[] = $path; } - if (!isset($options['recursive']) || $options['recursive']) { + if ($recursive) { $result = array_merge($result, static::findDirectories($path, $options)); } } @@ -505,7 +524,7 @@ public static function findDirectories(string $directory, array $options = []): * - recursive: boolean, whether the files under the subdirectories should also be looked for. Defaults to `true`. * * @psalm-param array{ - * filter?: \Yiisoft\Files\PathMatcher\PathMatcherInterface, + * filter?: \Yiisoft\Files\PathMatcher\PathMatcherInterface|mixed, * recursive?: bool, * } $options * @@ -520,11 +539,17 @@ public static function findFiles(string $directory, array $options = []): array throw new InvalidArgumentException("\"$directory\" is not a directory."); } - if (array_key_exists('filter', $options) && !$options['filter'] instanceof PathMatcherInterface) { - $type = is_object($options['filter']) ? get_class($options['filter']) : gettype($options['filter']); - throw new InvalidArgumentException(sprintf('Filter should be an instance of PathMatcherInterface, %s given.', $type)); + $filter = null; + if (array_key_exists('filter', $options)) { + if (!$options['filter'] instanceof PathMatcherInterface) { + $type = is_object($options['filter']) ? get_class($options['filter']) : gettype($options['filter']); + throw new InvalidArgumentException(sprintf('Filter should be an instance of PathMatcherInterface, %s given.', $type)); + } + $filter = $options['filter']; } + $recursive = !array_key_exists('recursive', $options) || $options['recursive']; + $directory = static::normalizePath($directory); $result = []; @@ -538,13 +563,13 @@ public static function findFiles(string $directory, array $options = []): array $path = $directory . '/' . $file; if (is_file($path)) { - if (!isset($options['filter']) || $options['filter']->match($path)) { + if ($filter === null || $filter->match($path)) { $result[] = $path; } continue; } - if (!isset($options['recursive']) || $options['recursive']) { + if ($recursive) { $result = array_merge($result, static::findFiles($path, $options)); } } From 03002678db7f7fa0fd342b0f8298b7f99321daed Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Thu, 4 Feb 2021 22:22:41 +0300 Subject: [PATCH 10/11] Fix test --- tests/FileHelperTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/FileHelperTest.php b/tests/FileHelperTest.php index 9903479..549fa98 100644 --- a/tests/FileHelperTest.php +++ b/tests/FileHelperTest.php @@ -51,7 +51,7 @@ public function testCreateDirectoryPermissions(): void $basePath = $this->testFilePath; $dirName = $basePath . '/test_dir_perms'; - $this->assertTrue(FileHelper::ensureDirectory($dirName, 0700)); + FileHelper::ensureDirectory($dirName, 0700); $this->assertFileMode(0700, $dirName); } From bc7b6961884ee00f085ef6e10bb589c49a5c09e1 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Thu, 4 Feb 2021 22:30:58 +0300 Subject: [PATCH 11/11] Fix/suppress Psalm errors --- src/FileHelper.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/FileHelper.php b/src/FileHelper.php index c37f93e..6562c89 100644 --- a/src/FileHelper.php +++ b/src/FileHelper.php @@ -39,6 +39,7 @@ class FileHelper */ public static function openFile(string $filename, string $mode, bool $useIncludePath = false, $context = null) { + /** @psalm-suppress InvalidArgument, MixedArgumentTypeCoercion */ set_error_handler(static function (int $errorNumber, string $errorString) use ($filename): ?bool { throw new RuntimeException( sprintf('Failed to open a file "%s". ', $filename) . $errorString, @@ -75,6 +76,7 @@ public static function ensureDirectory(string $path, int $mode = 0775): void $path = static::normalizePath($path); if (!is_dir($path)) { + /** @psalm-suppress InvalidArgument, MixedArgumentTypeCoercion */ set_error_handler(static function (int $errorNumber, string $errorString) use ($path) { // Handle race condition. // See https://github.com/kalessil/phpinspectionsea/blob/master/docs/probable-bugs.md#mkdir-race-condition @@ -162,7 +164,8 @@ public static function removeDirectory(string $directory, array $options = []): if (is_link($directory)) { self::unlink($directory); } else { - set_error_handler(static function (int $errorNumber, string $errorString, string $errorFile, int $errorLine, array $context) use ($directory) { + /** @psalm-suppress InvalidArgument, MixedArgumentTypeCoercion */ + set_error_handler(static function (int $errorNumber, string $errorString) use ($directory) { throw new RuntimeException( sprintf('Failed to remove directory "%s". ', $directory) . $errorString, $errorNumber @@ -213,6 +216,7 @@ public static function clearDirectory(string $directory, array $options = []): v */ public static function unlink(string $path): void { + /** @psalm-suppress InvalidArgument, MixedArgumentTypeCoercion */ set_error_handler(static function (int $errorNumber, string $errorString) use ($path) { throw new RuntimeException( sprintf('Failed to unlink "%s". ', $path) . $errorString, @@ -291,7 +295,7 @@ public static function isEmptyDirectory(string $path): bool * @psalm-param array{ * dirMode?: int, * fileMode?: int, - * filter?: \Yiisoft\Files\PathMatcher\PathMatcherInterface, + * filter?: \Yiisoft\Files\PathMatcher\PathMatcherInterface|mixed, * recursive?: bool, * beforeCopy?: callable, * afterCopy?: callable, @@ -386,6 +390,7 @@ private static function assertNotSelfDirectory(string $source, string $destinati */ private static function openDirectory(string $directory) { + /** @psalm-suppress InvalidArgument, MixedArgumentTypeCoercion */ set_error_handler(static function (int $errorNumber, string $errorString) use ($directory) { throw new RuntimeException( sprintf('Unable to open directory "%s". ', $directory) . $errorString,