MC-31362: Media folder update

- Added strict typing
- Added docblocks and type hints to tests
- Updated unit tests

Author: Hwashiang Yu

app/code/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFolder.php

@@ -4,6 +4,9 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
+declare(strict_types=1);
+
 namespace Magento\Cms\Controller\Adminhtml\Wysiwyg\Images;
 
 use Magento\Framework\App\Action\HttpPostActionInterface;

app/code/Magento/Theme/Model/Design/Backend/File.php

@@ -4,6 +4,8 @@
  * See COPYING.txt for license details.
  */
 
+declare(strict_types=1);
+
 namespace Magento\Theme\Model\Design\Backend;
 
 use Magento\Config\Model\Config\Backend\File as BackendFile;

app/code/Magento/Theme/Test/Unit/Model/Design/Backend/FileTest.php

@@ -3,41 +3,51 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
+declare(strict_types=1);
+
 namespace Magento\Theme\Test\Unit\Model\Design\Backend;
 
 use Magento\Framework\App\Filesystem\DirectoryList;
 use Magento\Framework\Exception\LocalizedException;
+use Magento\Framework\File\Mime;
+use Magento\Framework\Filesystem\Directory\WriteInterface;
 use Magento\Framework\Filesystem\Io\File as IoFileSystem;
 use Magento\Framework\UrlInterface;
+use Magento\MediaStorage\Helper\File\Storage\Database;
 use Magento\Theme\Model\Design\Backend\File;
+use PHPUnit_Framework_MockObject_MockObject;
 
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class FileTest extends \PHPUnit\Framework\TestCase
 {
-    /** @var \Magento\Framework\Filesystem\Directory\WriteInterface|\PHPUnit_Framework_MockObject_MockObject */
-    protected $mediaDirectory;
+    /** @var WriteInterface|PHPUnit_Framework_MockObject_MockObject */
+    private $mediaDirectory;
 
-    /** @var UrlInterface|\PHPUnit_Framework_MockObject_MockObject */
-    protected $urlBuilder;
+    /** @var UrlInterface|PHPUnit_Framework_MockObject_MockObject */
+    private $urlBuilder;
 
     /** @var File */
-    protected $fileBackend;
+    private $fileBackend;
 
-    /** @var IoFileSystem */
+    /** @var IoFileSystem|PHPUnit_Framework_MockObject_MockObject */
     private $ioFileSystem;
 
     /**
-     * @var \Magento\Framework\File\Mime|\PHPUnit_Framework_MockObject_MockObject
+     * @var Mime|PHPUnit_Framework_MockObject_MockObject
      */
     private $mime;
 
     /**
-     * @var \Magento\MediaStorage\Helper\File\Storage\Database|\PHPUnit_Framework_MockObject_MockObject
+     * @var Database|PHPUnit_Framework_MockObject_MockObject
      */
     private $databaseHelper;
 
+    /**
+     * @inheritdoc
+     */
     public function setUp()
     {
         $context = $this->getMockObject(\Magento\Framework\Model\Context::class);
@@ -59,7 +69,7 @@ public function setUp()
             ->disableOriginalConstructor()
             ->getMock();
         $this->mediaDirectory = $this->getMockBuilder(
-            \Magento\Framework\Filesystem\Directory\WriteInterface::class
+            WriteInterface::class
         )
             ->getMockForAbstractClass();
         $filesystem->expects($this->once())
@@ -68,11 +78,13 @@ public function setUp()
             ->willReturn($this->mediaDirectory);
         $this->urlBuilder = $this->getMockBuilder(\Magento\Framework\UrlInterface::class)
             ->getMockForAbstractClass();
-        $this->mime = $this->getMockBuilder(\Magento\Framework\File\Mime::class)
+        $this->ioFileSystem = $this->getMockBuilder(\Magento\Framework\Filesystem\Io\File::class)
+            ->getMockForAbstractClass();
+        $this->mime = $this->getMockBuilder(Mime::class)
             ->disableOriginalConstructor()
             ->getMock();
         $this->databaseHelper = $this->getMockBuilder(
-            \Magento\MediaStorage\Helper\File\Storage\Database::class
+            Database::class
         )
             ->disableOriginalConstructor()
             ->getMock();
@@ -83,7 +95,6 @@ public function setUp()
         $abstractDb = $this->getMockBuilder(\Magento\Framework\Data\Collection\AbstractDb::class)
             ->disableOriginalConstructor()
             ->getMockForAbstractClass();
-        $this->ioFileSystem = new IoFileSystem();
         $this->fileBackend = new File(
             $context,
             $registry,
@@ -107,17 +118,22 @@ public function setUp()
         );
     }
 
+    /**
+     * @inheritdoc
+     */
     public function tearDown()
     {
         unset($this->fileBackend);
     }
 
     /**
+     * Gets the mock object.
+     *
      * @param string $class
      * @param array $methods
-     * @return \PHPUnit_Framework_MockObject_MockObject
+     * @return PHPUnit_Framework_MockObject_MockObject
      */
-    protected function getMockObject($class, $methods = [])
+    private function getMockObject(string $class, array $methods = []): PHPUnit_Framework_MockObject_MockObject
     {
         $builder =  $this->getMockBuilder($class)
             ->disableOriginalConstructor();
@@ -128,15 +144,20 @@ protected function getMockObject($class, $methods = [])
     }
 
     /**
+     * Gets mock objects for abstract class.
+     *
      * @param string $class
-     * @return \PHPUnit_Framework_MockObject_MockObject
+     * @return PHPUnit_Framework_MockObject_MockObject
      */
-    protected function getMockObjectForAbstractClass($class)
+    private function getMockObjectForAbstractClass(string $class): PHPUnit_Framework_MockObject_MockObject
     {
         return  $this->getMockBuilder($class)
             ->getMockForAbstractClass();
     }
 
+    /**
+     * Test for afterLoad method.
+     */
     public function testAfterLoad()
     {
         $value = 'filename.jpg';
@@ -201,11 +222,13 @@ public function testAfterLoad()
     }
 
     /**
+     * Test for beforeSave method.
+     *
      * @dataProvider beforeSaveDataProvider
      * @param string $fileName
      * @throws LocalizedException
      */
-    public function testBeforeSave($fileName)
+    public function testBeforeSave(string $fileName)
     {
         $expectedFileName = basename($fileName);
         $expectedTmpMediaPath = 'tmp/design/file/' . $expectedFileName;
@@ -247,17 +270,21 @@ public function testBeforeSave($fileName)
     }
 
     /**
+     * Data provider for testBeforeSave.
+     *
      * @return array
      */
-    public function beforeSaveDataProvider()
+    public function beforeSaveDataProvider(): array
     {
         return [
             'Normal file name' => ['filename.jpg'],
-            'Vulnerable file name' => ['../../../../../../../../etc/passwd'],
+            'Vulnerable file name' => ['../../../../../../../../etc/pass'],
         ];
     }
 
     /**
+     * Test for beforeSave method without file.
+     *
      * @expectedException \Magento\Framework\Exception\LocalizedException
      * @expectedExceptionMessage header_logo_src does not contain field 'file'
      */
@@ -276,6 +303,11 @@ public function testBeforeSaveWithoutFile()
         $this->fileBackend->beforeSave();
     }
 
+    /**
+     * Test for beforeSave method with existing file.
+     *
+     * @throws LocalizedException
+     */
     public function testBeforeSaveWithExistingFile()
     {
         $value = 'filename.jpg';
@@ -327,7 +359,7 @@ public function getRelativeMediaPathDataProvider(): array
     {
         return [
             'Normal path' => ['pub/media/', 'filename.jpg'],
-            'Complex path' => ['somepath/pub/media/', 'filename.jpg'],
+            'Complex path' => ['some_path/pub/media/', 'filename.jpg'],
         ];
     }
 }

app/code/Magento/Theme/Test/Unit/Model/Design/Backend/ImageTest.php

@@ -3,19 +3,25 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
+declare(strict_types=1);
+
 namespace Magento\Theme\Test\Unit\Model\Design\Backend;
 
-use Magento\Framework\Filesystem\Io\File as IoFileSystem;
 use Magento\Theme\Model\Design\Backend\Image;
+use PHPUnit_Framework_MockObject_MockObject;
 
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class ImageTest extends \PHPUnit\Framework\TestCase
 {
     /** @var Image */
-    protected $imageBackend;
+    private $imageBackend;
 
+    /**
+     * @inheritdoc
+     */
     public function setUp()
     {
         $context = $this->getMockObject(\Magento\Framework\Model\Context::class);
@@ -31,7 +37,7 @@ public function setUp()
         $databaseHelper = $this->getMockObject(\Magento\MediaStorage\Helper\File\Storage\Database::class);
         $abstractResource = $this->getMockObject(\Magento\Framework\Model\ResourceModel\AbstractResource::class);
         $abstractDb = $this->getMockObject(\Magento\Framework\Data\Collection\AbstractDb::class);
-        $ioFileSystem = new IoFileSystem();
+        $ioFileSystem = $this->getMockObject(\Magento\Framework\Filesystem\Io\File::class);
         $this->imageBackend = new Image(
             $context,
             $registry,
@@ -49,6 +55,9 @@ public function setUp()
         );
     }
 
+    /**
+     * @inheritdoc
+     */
     public function tearDown()
     {
         unset($this->imageBackend);
@@ -57,9 +66,9 @@ public function tearDown()
     /**
      * @param string $class
      * @param array $methods
-     * @return \PHPUnit_Framework_MockObject_MockObject
+     * @return PHPUnit_Framework_MockObject_MockObject
      */
-    protected function getMockObject($class, $methods = [])
+    private function getMockObject(string $class, array $methods = []): PHPUnit_Framework_MockObject_MockObject
     {
         $builder =  $this->getMockBuilder($class)
             ->disableOriginalConstructor();
@@ -70,6 +79,8 @@ protected function getMockObject($class, $methods = [])
     }
 
     /**
+     * Test for beforeSave method with invalid file extension.
+     *
      * @expectedException \Magento\Framework\Exception\LocalizedException
      * @expectedExceptionMessage Something is wrong with the file upload settings.
      */

lib/internal/Magento/Framework/Filesystem/Test/Unit/Driver/FileTest.php

@@ -4,6 +4,8 @@
  * See COPYING.txt for license details.
  */
 
+declare(strict_types=1);
+
 namespace Magento\Framework\Filesystem\Test\Unit\Driver;
 
 use Magento\Framework\Filesystem\Driver\File;
@@ -16,28 +18,35 @@ class FileTest extends \PHPUnit\Framework\TestCase
     /** @var bool Result of file_put_contents() function */
     public static $filePutContents;
 
+    /**
+     * @inheritdoc
+     */
     public function setUp()
     {
         self::$fileGetContents = '';
         self::$filePutContents = true;
     }
 
     /**
+     * Test for getAbsolutePath method.
+     *
      * @dataProvider dataProviderForTestGetAbsolutePath
-     * @param $basePath
-     * @param $path
-     * @param $expected
+     * @param string $basePath
+     * @param string $path
+     * @param string $expected
      */
-    public function testGetAbsolutePath($basePath, $path, $expected)
+    public function testGetAbsolutePath(string $basePath, string $path, string $expected)
     {
         $file = new File();
         $this->assertEquals($expected, $file->getAbsolutePath($basePath, $path));
     }
 
     /**
+     * Data provider for testGetAbsolutePath.
+     *
      * @return array
      */
-    public function dataProviderForTestGetAbsolutePath()
+    public function dataProviderForTestGetAbsolutePath(): array
     {
         return [
             ['/root/path/', 'sub', '/root/path/sub'],
@@ -48,21 +57,25 @@ public function dataProviderForTestGetAbsolutePath()
     }
 
     /**
+     * Test for getRelativePath method.
+     *
      * @dataProvider dataProviderForTestGetRelativePath
-     * @param $basePath
-     * @param $path
-     * @param $expected
+     * @param string $basePath
+     * @param string $path
+     * @param string $expected
      */
-    public function testGetRelativePath($basePath, $path, $expected)
+    public function testGetRelativePath(string $basePath, string $path, string $expected)
     {
         $file = new File();
         $this->assertEquals($expected, $file->getRelativePath($basePath, $path));
     }
 
     /**
+     * Data provider for testGetRelativePath.
+     *
      * @return array
      */
-    public function dataProviderForTestGetRelativePath()
+    public function dataProviderForTestGetRelativePath(): array
     {
         return [
             ['/root/path/', 'sub', 'sub'],
@@ -73,20 +86,24 @@ public function dataProviderForTestGetRelativePath()
     }
 
     /**
-     * @dataProvider dataProviderForTestRealPathSafety
-     * @param $path
-     * @param $expected
+     * Test for getRealPathSafety method.
+     *
+     * @dataProvider dataProviderForTestGetRealPathSafety
+     * @param string $path
+     * @param string $expected
      */
-    public function testGetRealPathSafety($path, $expected)
+    public function testGetRealPathSafety(string $path, string $expected)
     {
         $file = new File();
         $this->assertEquals($expected, $file->getRealPathSafety($path));
     }
 
     /**
+     * Data provider for testGetRealPathSafety;
+     *
      * @return array
      */
-    public function dataProviderForTestRealPathSafety()
+    public function dataProviderForTestGetRealPathSafety(): array
     {
         return [
             ['/1/2/3', '/1/2/3'],