Added copyEmptyDirectories option to FileHelper

also set it to false in AssetManager to avoid creating a lot of empty directories. fixes yiisoft#9669
lirnbo · May 7, 2017 · 5e8e684 · 5e8e684
1 parent 583a2bc
commit 5e8e684
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 5 deletions.
diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md
@@ -7,6 +7,7 @@ Yii Framework 2 Change Log
 - Bug #4408: Add support for unicode word characters and `+` character in attribute names (sammousa, kmindi)
 - Bug #7946: Fixed a bug when the `form` attribute was not propagated to the hidden input of the checkbox (Kolyunya)
 - Bug #8120: Fixes LIKE special characters escaping for Cubrid/MSSQL/Oracle/SQLite in `yii\db\QueryBuilder` (sergeymakinen)
+- Bug #9669: AssetManager and `FileHelper::copyDirectory()` were copying empty directories when using `only` or `except` options. Added an option to disable this (cebe)
 - Bug #10346: Fixed "DOMException: Invalid Character Error" in `yii\web\XmlResponseFormatter::buildXml()` (sasha-ch)
 - Bug #10372: Fixed console controller including complex typed arguments in help (sammousa)
 - Bug #11230: Include `defaultRoles` in `yii\rbac\DbManager->getRolesByUser()` results (developeruz)

diff --git a/framework/helpers/BaseFileHelper.php b/framework/helpers/BaseFileHelper.php
@@ -249,6 +249,10 @@ protected static function loadMimeTypes($magicFile)
      * - afterCopy: callback, a PHP callback that is called after each sub-directory or file is successfully copied.
      *   The signature of the callback should be: `function ($from, $to)`, where `$from` is the sub-directory or
      *   file copied from, while `$to` is the copy target.
+     * - copyEmptyDirectories: boolean, whether to copy empty directories. Set this to false to avoid creating directories
+     *   that do not contain files. This affects directories that do not contain files initially as well as directories that
+     *   do not contain files at the target destination because files have been filtered via `only` or `except`.
+     *   Defaults to true. This option is available since version 2.0.12. Before 2.0.12 empty directories are always copied.
      * @throws \yii\base\InvalidParamException if unable to open directory
      */
     public static function copyDirectory($src, $dst, $options = [])
@@ -259,8 +263,10 @@ public static function copyDirectory($src, $dst, $options = [])
         if ($src === $dst || strpos($dst, $src . DIRECTORY_SEPARATOR) === 0) {
             throw new InvalidParamException('Trying to copy a directory to itself or a subdirectory.');
         }
-        if (!is_dir($dst)) {
+        $dstExists = is_dir($dst);
+        if (!$dstExists && (!isset($options['copyEmptyDirectories']) || $options['copyEmptyDirectories'])) {
             static::createDirectory($dst, isset($options['dirMode']) ? $options['dirMode'] : 0775, true);
+            $dstExists = true;
         }
 
         $handle = opendir($src);
@@ -283,6 +289,11 @@ public static function copyDirectory($src, $dst, $options = [])
                     continue;
                 }
                 if (is_file($from)) {
+                    if (!$dstExists) {
+                        // delay creation of destination directory until the first file is copied to avoid creating empty directories
+                        static::createDirectory($dst, isset($options['dirMode']) ? $options['dirMode'] : 0775, true);
+                        $dstExists = true;
+                    }
                     copy($from, $to);
                     if (isset($options['fileMode'])) {
                         @chmod($to, $options['fileMode']);

diff --git a/framework/web/AssetManager.php b/framework/web/AssetManager.php
@@ -527,6 +527,7 @@ protected function publishDirectory($src, $options)
                 [
                     'dirMode' => $this->dirMode,
                     'fileMode' => $this->fileMode,
+                    'copyEmptyDirectories' => false,
                 ]
             );
             if (!isset($opts['beforeCopy'])) {

diff --git a/tests/framework/helpers/FileHelperTest.php b/tests/framework/helpers/FileHelperTest.php
@@ -790,4 +790,59 @@ public function testCopyDirectoryExclude()
             $this->assertStringEqualsFile($fileName, $content, 'Incorrect file content!');
         }
     }
+
+    /**
+     * @see https://github.com/yiisoft/yii2/issues/9669
+     *
+     * @depends testCopyDirectory
+     * @depends testFindFiles
+     */
+    public function testCopyDirectoryEmptyDirectories()
+    {
+        $srcDirName = 'test_empty_src_dir';
+        $this->createFileStructure([
+            $srcDirName => [
+                'dir1' => [
+                    'file1.txt' => 'file1',
+                    'file2.txt' => 'file2',
+                ],
+                'dir2' => [
+                    'file1.log' => 'file1',
+                    'file2.log' => 'file2',
+                ],
+                'dir3' => [],
+            ],
+        ]);
+
+        $basePath = $this->testFilePath;
+        $srcDirName = $basePath . DIRECTORY_SEPARATOR . $srcDirName;
+
+        // copy with empty directories
+        $dstDirName = $basePath . DIRECTORY_SEPARATOR . 'test_empty_dst_dir';
+        FileHelper::copyDirectory($srcDirName, $dstDirName, ['only' => ['*.txt'], 'copyEmptyDirectories' => true]);
+
+        $this->assertFileExists($dstDirName, 'Destination directory does not exist!');
+        $copiedFiles = FileHelper::findFiles($dstDirName);
+        $this->assertCount(2, $copiedFiles, 'wrong files count copied');
+
+        $this->assertFileExists($dstDirName . DIRECTORY_SEPARATOR . 'dir1');
+        $this->assertFileExists($dstDirName . DIRECTORY_SEPARATOR . 'dir1' . DIRECTORY_SEPARATOR . 'file1.txt');
+        $this->assertFileExists($dstDirName . DIRECTORY_SEPARATOR . 'dir1' . DIRECTORY_SEPARATOR . 'file2.txt');
+        $this->assertFileExists($dstDirName . DIRECTORY_SEPARATOR . 'dir2');
+        $this->assertFileExists($dstDirName . DIRECTORY_SEPARATOR . 'dir3');
+
+        // copy without empty directories
+        $dstDirName = $basePath . DIRECTORY_SEPARATOR . 'test_empty_dst_dir2';
+        FileHelper::copyDirectory($srcDirName, $dstDirName, ['only' => ['*.txt'], 'copyEmptyDirectories' => false]);
+
+        $this->assertFileExists($dstDirName, 'Destination directory does not exist!');
+        $copiedFiles = FileHelper::findFiles($dstDirName);
+        $this->assertCount(2, $copiedFiles, 'wrong files count copied');
+
+        $this->assertFileExists($dstDirName . DIRECTORY_SEPARATOR . 'dir1');
+        $this->assertFileExists($dstDirName . DIRECTORY_SEPARATOR . 'dir1' . DIRECTORY_SEPARATOR . 'file1.txt');
+        $this->assertFileExists($dstDirName . DIRECTORY_SEPARATOR . 'dir1' . DIRECTORY_SEPARATOR . 'file2.txt');
+        $this->assertFileNotExists($dstDirName . DIRECTORY_SEPARATOR . 'dir2');
+        $this->assertFileNotExists($dstDirName . DIRECTORY_SEPARATOR . 'dir3');
+    }
 }
diff --git a/tests/framework/web/AssetBundleTest.php b/tests/framework/web/AssetBundleTest.php
@@ -8,6 +8,7 @@
 namespace yiiunit\framework\web;
 
 use Yii;
+use yii\helpers\FileHelper;
 use yii\web\View;
 use yii\web\AssetBundle;
 use yii\web\AssetManager;
@@ -27,6 +28,24 @@ protected function setUp()
         Yii::setAlias('@testAssetsPath', '@webroot/assets');
         Yii::setAlias('@testAssetsUrl', '@web/assets');
         Yii::setAlias('@testSourcePath', '@webroot/assetSources');
+
+        // clean up assets directory
+        $handle = opendir($dir = Yii::getAlias('@testAssetsPath'));
+        if ($handle === false) {
+            throw new \Exception("Unable to open directory: $dir");
+        }
+        while (($file = readdir($handle)) !== false) {
+            if ($file === '.' || $file === '..' || $file === '.gitignore') {
+                continue;
+            }
+            $path = $dir . DIRECTORY_SEPARATOR . $file;
+            if (is_dir($path)) {
+                FileHelper::removeDirectory($path);
+            } else {
+                unlink($path);
+            }
+        }
+        closedir($handle);
     }
 
     /**
@@ -105,12 +124,11 @@ public function testSourcesPublish_AssetManagerBeforeCopy()
         $bundle = TestSourceAsset::register($view);
         $bundle->publish($am);
 
-        $this->assertTrue(is_dir($bundle->basePath));
+        $this->assertFalse(is_dir($bundle->basePath));
         foreach ($bundle->js as $filename) {
             $publishedFile = $bundle->basePath . DIRECTORY_SEPARATOR . $filename;
             $this->assertFileNotExists($publishedFile);
         }
-        $this->assertTrue(rmdir($bundle->basePath));
     }
 
     public function testSourcesPublish_AssetBeforeCopy()
@@ -126,12 +144,11 @@ public function testSourcesPublish_AssetBeforeCopy()
         ];
         $bundle->publish($am);
 
-        $this->assertTrue(is_dir($bundle->basePath));
+        $this->assertFalse(is_dir($bundle->basePath));
         foreach ($bundle->js as $filename) {
             $publishedFile = $bundle->basePath . DIRECTORY_SEPARATOR . $filename;
             $this->assertFileNotExists($publishedFile);
         }
-        $this->assertTrue(rmdir($bundle->basePath));
     }
 
     public function testSourcesPublish_publishOptions_Only()