Merge remote-tracking branch 'origin/MC-17765' into 2.3-develop-pr59

viktorpetryk · viktorpetryk · commit 241b77d8fb04 · 2019-07-31T12:14:36.000+03:00
diff --git a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php
@@ -6,6 +6,7 @@
 namespace Magento\Catalog\Model\Category\Attribute\Backend;
 
 use Magento\Framework\App\Filesystem\DirectoryList;
+use Magento\Framework\File\Uploader;
 
 /**
  * Catalog category image attribute backend model
@@ -71,7 +72,7 @@ public function __construct(
     /**
      * Gets image name from $value array.
      *
-     * Will return empty string in a case when $value is not an array
+     * Will return empty string in a case when $value is not an array.
      *
      * @param array $value Attribute value
      * @return string
@@ -86,9 +87,28 @@ private function getUploadedImageName($value)
     }
 
     /**
-     * Avoiding saving potential upload data to DB
+     * Check that image name exists in catalog/category directory and return new image name if it already exists.
      *
-     * Will set empty image attribute value if image was not uploaded
+     * @param string $imageName
+     * @return string
+     */
+    private function checkUniqueImageName(string $imageName): string
+    {
+        $imageUploader = $this->getImageUploader();
+        $mediaDirectory = $this->_filesystem->getDirectoryWrite(DirectoryList::MEDIA);
+        $imageAbsolutePath = $mediaDirectory->getAbsolutePath(
+            $imageUploader->getBasePath() . DIRECTORY_SEPARATOR . $imageName
+        );
+
+        $imageName = Uploader::getNewFilename($imageAbsolutePath);
+
+        return $imageName;
+    }
+
+    /**
+     * Avoiding saving potential upload data to DB.
+     *
+     * Will set empty image attribute value if image was not uploaded.
      *
      * @param \Magento\Framework\DataObject $object
      * @return $this
@@ -105,6 +125,7 @@ public function beforeSave($object)
         }
 
         if ($imageName = $this->getUploadedImageName($value)) {
+            $imageName = $this->checkUniqueImageName($imageName);
             $object->setData($this->additionalData . $attributeName, $value);
             $object->setData($attributeName, $imageName);
         } elseif (!is_string($value)) {
@@ -160,6 +181,7 @@ private function fileResidesOutsideCategoryDir($value)
         if (!$baseMediaDir) {
             return false;
         }
+
         return strpos($fileUrl, $baseMediaDir) === 0;
     }
 
diff --git a/app/code/Magento/Catalog/Model/ImageUploader.php b/app/code/Magento/Catalog/Model/ImageUploader.php
@@ -5,6 +5,8 @@
  */
 namespace Magento\Catalog\Model;
 
+use Magento\Framework\File\Uploader;
+
 /**
  * Catalog image uploader
  */
@@ -199,7 +201,14 @@ public function moveFileFromTmp($imageName)
         $baseTmpPath = $this->getBaseTmpPath();
         $basePath = $this->getBasePath();
 
-        $baseImagePath = $this->getFilePath($basePath, $imageName);
+        $baseImagePath = $this->getFilePath(
+            $basePath,
+            Uploader::getNewFileName(
+                $this->mediaDirectory->getAbsolutePath(
+                    $this->getFilePath($basePath, $imageName)
+                )
+            )
+        );
         $baseTmpImagePath = $this->getFilePath($baseTmpPath, $imageName);
 
         try {
diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Category/Attribute/Backend/ImageTest.php
@@ -6,7 +6,12 @@
 namespace Magento\Catalog\Test\Unit\Model\Category\Attribute\Backend;
 
 use Magento\Framework\App\Filesystem\DirectoryList;
+use Magento\Framework\Filesystem\Directory\WriteInterface;
 
+/**
+ * Test for Magento\Catalog\Model\Category\Attribute\Backend\Image class.
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
+ */
 class ImageTest extends \PHPUnit\Framework\TestCase
 {
     /**
@@ -67,7 +72,7 @@ protected function setUp()
 
         $this->imageUploader = $this->createPartialMock(
             \Magento\Catalog\Model\ImageUploader::class,
-            ['moveFileFromTmp']
+            ['moveFileFromTmp', 'getBasePath']
         );
 
         $this->filesystem = $this->getMockBuilder(\Magento\Framework\Filesystem::class)->disableOriginalConstructor()
@@ -95,9 +100,7 @@ public function testBeforeSaveValueDeletion($value)
         $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class);
         $model->setAttribute($this->attribute);
 
-        $object = new \Magento\Framework\DataObject([
-            'test_attribute' => $value
-        ]);
+        $object = new \Magento\Framework\DataObject(['test_attribute' => $value]);
 
         $model->beforeSave($object);
 
@@ -132,9 +135,7 @@ public function testBeforeSaveValueInvalid($value)
         $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class);
         $model->setAttribute($this->attribute);
 
-        $object = new \Magento\Framework\DataObject([
-            'test_attribute' => $value
-        ]);
+        $object = new \Magento\Framework\DataObject(['test_attribute' => $value]);
 
         $model->beforeSave($object);
 
@@ -146,14 +147,25 @@ public function testBeforeSaveValueInvalid($value)
      */
     public function testBeforeSaveAttributeFileName()
     {
-        $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class);
-        $model->setAttribute($this->attribute);
+        $model = $this->setUpModelForAfterSave();
+        $mediaDirectoryMock = $this->createMock(WriteInterface::class);
+        $this->filesystem->expects($this->once())
+            ->method('getDirectoryWrite')
+            ->with(DirectoryList::MEDIA)
+            ->willReturn($mediaDirectoryMock);
+        $this->imageUploader->expects($this->once())->method('getBasePath')->willReturn('base/path');
+        $mediaDirectoryMock->expects($this->once())
+            ->method('getAbsolutePath')
+            ->with('base/path/test123.jpg')
+            ->willReturn('absolute/path/base/path/test123.jpg');
 
-        $object = new \Magento\Framework\DataObject([
-            'test_attribute' => [
-                ['name' => 'test123.jpg']
+        $object = new \Magento\Framework\DataObject(
+            [
+                'test_attribute' => [
+                    ['name' => 'test123.jpg'],
+                ],
             ]
-        ]);
+        );
 
         $model->beforeSave($object);
 
@@ -165,30 +177,37 @@ public function testBeforeSaveAttributeFileName()
      */
     public function testBeforeSaveAttributeFileNameOutsideOfCategoryDir()
     {
-        $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class, [
-            'filesystem' => $this->filesystem
-        ]);
-
+        $model = $this->setUpModelForAfterSave();
         $model->setAttribute($this->attribute);
 
+        $mediaDirectoryMock = $this->createMock(WriteInterface::class);
+        $this->filesystem->expects($this->once())
+            ->method('getDirectoryWrite')
+            ->with(DirectoryList::MEDIA)
+            ->willReturn($mediaDirectoryMock);
         $this->filesystem
             ->expects($this->once())
             ->method('getUri')
             ->with(DirectoryList::MEDIA)
             ->willReturn('pub/media');
+        $mediaDirectoryMock->expects($this->once())
+            ->method('getAbsolutePath')
+            ->willReturn('/pub/media/wysiwyg/test123.jpg');
 
-        $object = new \Magento\Framework\DataObject([
-            'test_attribute' => [
-                [
-                    'name' => '/test123.jpg',
-                    'url' => '/pub/media/wysiwyg/test123.jpg',
-                ]
+        $object = new \Magento\Framework\DataObject(
+            [
+                'test_attribute' => [
+                    [
+                        'name' => 'test123.jpg',
+                        'url' => '/pub/media/wysiwyg/test123.jpg',
+                    ],
+                ],
             ]
-        ]);
+        );
 
         $model->beforeSave($object);
 
-        $this->assertEquals('/pub/media/wysiwyg/test123.jpg', $object->getTestAttribute());
+        $this->assertEquals('test123.jpg', $object->getTestAttribute());
         $this->assertEquals(
             [['name' => '/pub/media/wysiwyg/test123.jpg', 'url' => '/pub/media/wysiwyg/test123.jpg']],
             $object->getData('_additional_data_test_attribute')
@@ -200,20 +219,31 @@ public function testBeforeSaveAttributeFileNameOutsideOfCategoryDir()
      */
     public function testBeforeSaveTemporaryAttribute()
     {
-        $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class);
+        $model = $this->setUpModelForAfterSave();
         $model->setAttribute($this->attribute);
 
-        $object = new \Magento\Framework\DataObject([
-            'test_attribute' => [
-                ['name' => 'test123.jpg', 'tmp_name' => 'abc123', 'url' => 'http://www.example.com/test123.jpg']
+        $mediaDirectoryMock = $this->createMock(WriteInterface::class);
+        $this->filesystem->expects($this->once())
+            ->method('getDirectoryWrite')
+            ->with(DirectoryList::MEDIA)
+            ->willReturn($mediaDirectoryMock);
+
+        $object = new \Magento\Framework\DataObject(
+            [
+                'test_attribute' => [
+                    ['name' => 'test123.jpg', 'tmp_name' => 'abc123', 'url' => 'http://www.example.com/test123.jpg'],
+                ],
             ]
-        ]);
+        );
 
         $model->beforeSave($object);
 
-        $this->assertEquals([
-            ['name' => 'test123.jpg', 'tmp_name' => 'abc123', 'url' => 'http://www.example.com/test123.jpg']
-        ], $object->getData('_additional_data_test_attribute'));
+        $this->assertEquals(
+            [
+                ['name' => 'test123.jpg', 'tmp_name' => 'abc123', 'url' => 'http://www.example.com/test123.jpg'],
+            ],
+            $object->getData('_additional_data_test_attribute')
+        );
     }
 
     /**
@@ -224,9 +254,7 @@ public function testBeforeSaveAttributeStringValue()
         $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class);
         $model->setAttribute($this->attribute);
 
-        $object = new \Magento\Framework\DataObject([
-            'test_attribute' => 'test123.jpg'
-        ]);
+        $object = new \Magento\Framework\DataObject(['test_attribute' => 'test123.jpg']);
 
         $model->beforeSave($object);
 
@@ -245,18 +273,26 @@ private function setUpModelForAfterSave()
 
         $objectManagerMock->expects($this->any())
             ->method('get')
-            ->will($this->returnCallback(function ($class, $params = []) use ($imageUploaderMock) {
-                if ($class == \Magento\Catalog\CategoryImageUpload::class) {
-                    return $imageUploaderMock;
-                }
-
-                return $this->objectManager->get($class, $params);
-            }));
-
-        $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class, [
-            'objectManager' => $objectManagerMock,
-            'logger' => $this->logger
-        ]);
+            ->will(
+                $this->returnCallback(
+                    function ($class, $params = []) use ($imageUploaderMock) {
+                        if ($class == \Magento\Catalog\CategoryImageUpload::class) {
+                            return $imageUploaderMock;
+                        }
+
+                        return $this->objectManager->get($class, $params);
+                    }
+                )
+            );
+
+        $model = $this->objectManager->getObject(
+            \Magento\Catalog\Model\Category\Attribute\Backend\Image::class,
+            [
+                'objectManager' => $objectManagerMock,
+                'logger' => $this->logger,
+                'filesystem' => $this->filesystem,
+            ]
+        );
         $this->objectManager->setBackwardCompatibleProperty($model, 'imageUploader', $this->imageUploader);
 
         return $model->setAttribute($this->attribute);
diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Model/ImageUploaderTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Model/ImageUploaderTest.php
@@ -47,8 +47,8 @@ protected function setUp()
         $this->imageUploader = $this->objectManager->create(
             \Magento\Catalog\Model\ImageUploader::class,
             [
-                'baseTmpPath' => $this->mediaDirectory->getRelativePath('tmp'),
-                'basePath' => __DIR__,
+                'baseTmpPath' => 'catalog/tmp/category',
+                'basePath' => 'catalog/category',
                 'allowedExtensions' => ['jpg', 'jpeg', 'gif', 'png'],
                 'allowedMimeTypes' => ['image/jpg', 'image/jpeg', 'image/gif', 'image/png']
             ]
@@ -79,6 +79,24 @@ public function testSaveFileToTmpDir(): void
         $this->assertTrue(is_file($this->mediaDirectory->getAbsolutePath($filePath)));
     }
 
+    /**
+     * Test that method rename files when move it with the same name into base directory.
+     *
+     * @return void
+     * @magentoDataFixture Magento/Catalog/_files/catalog_category_image.php
+     * @magentoDataFixture Magento/Catalog/_files/catalog_tmp_category_image.php
+     */
+    public function testMoveFileFromTmp(): void
+    {
+        $expectedFilePath = $this->imageUploader->getBasePath() . DIRECTORY_SEPARATOR . 'magento_small_image_1.jpg';
+
+        $this->assertFileNotExists($this->mediaDirectory->getAbsolutePath($expectedFilePath));
+
+        $this->imageUploader->moveFileFromTmp('magento_small_image.jpg');
+
+        $this->assertFileExists($this->mediaDirectory->getAbsolutePath($expectedFilePath));
+    }
+
     /**
      * @expectedException \Magento\Framework\Exception\LocalizedException
      * @expectedExceptionMessage File validation failed.
diff --git a/dev/tests/integration/testsuite/Magento/Catalog/_files/catalog_category_image.php b/dev/tests/integration/testsuite/Magento/Catalog/_files/catalog_category_image.php
@@ -0,0 +1,19 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+use Magento\Framework\App\Filesystem\DirectoryList;
+
+$objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
+
+/** @var $mediaDirectory \Magento\Framework\Filesystem\Directory\WriteInterface */
+$mediaDirectory = $objectManager->get(\Magento\Framework\Filesystem::class)
+    ->getDirectoryWrite(DirectoryList::MEDIA);
+$fileName = 'magento_small_image.jpg';
+$filePath = 'catalog/category/' . $fileName;
+$mediaDirectory->create('catalog/category');
+
+copy(__DIR__ . DIRECTORY_SEPARATOR . $fileName, $mediaDirectory->getAbsolutePath($filePath));
diff --git a/dev/tests/integration/testsuite/Magento/Catalog/_files/catalog_category_image_rollback.php b/dev/tests/integration/testsuite/Magento/Catalog/_files/catalog_category_image_rollback.php
@@ -0,0 +1,17 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+use Magento\Framework\App\Filesystem\DirectoryList;
+
+/** @var \Magento\Framework\Filesystem\Directory\WriteInterface $mediaDirectory */
+$mediaDirectory = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
+    \Magento\Framework\Filesystem::class
+)->getDirectoryWrite(
+    DirectoryList::MEDIA
+);
+
+$mediaDirectory->delete('catalog/category');
diff --git a/dev/tests/integration/testsuite/Magento/Catalog/_files/catalog_tmp_category_image.php b/dev/tests/integration/testsuite/Magento/Catalog/_files/catalog_tmp_category_image.php
@@ -0,0 +1,19 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+use Magento\Framework\App\Filesystem\DirectoryList;
+
+$objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
+
+/** @var $mediaDirectory \Magento\Framework\Filesystem\Directory\WriteInterface */
+$mediaDirectory = $objectManager->get(\Magento\Framework\Filesystem::class)
+    ->getDirectoryWrite(DirectoryList::MEDIA);
+$fileName = 'magento_small_image.jpg';
+$tmpFilePath = 'catalog/tmp/category/' . $fileName;
+$mediaDirectory->create('catalog/tmp/category');
+
+copy(__DIR__ . DIRECTORY_SEPARATOR . $fileName, $mediaDirectory->getAbsolutePath($tmpFilePath));
diff --git a/dev/tests/integration/testsuite/Magento/Catalog/_files/catalog_tmp_category_image_rollback.php b/dev/tests/integration/testsuite/Magento/Catalog/_files/catalog_tmp_category_image_rollback.php
