diff --git a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php index cd450e26cd832..86ef9d61c527c 100644 --- a/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php +++ b/app/code/Magento/Catalog/Model/Category/Attribute/Backend/Image.php @@ -70,6 +70,7 @@ public function __construct( /** * Gets image name from $value array. + * * Will return empty string in a case when $value is not an array * * @param array $value Attribute value @@ -85,7 +86,23 @@ private function getUploadedImageName($value) } /** - * Avoiding saving potential upload data to DB + * Get the destination image name which prevents overwriting existing images with the same name + * + * @param array $value Attribute value + * @return string + */ + private function getFinalImageName($value) + { + if (is_array($value) && isset($value[0]['name'])) { + return $this->getImageUploader()->getFinalImageName($value[0]['name']); + } + + return ''; + } + + /** + * Avoiding saving potential upload data to DB. + * * Will set empty image attribute value if image was not uploaded * * @param \Magento\Framework\DataObject $object @@ -102,7 +119,7 @@ public function beforeSave($object) $value[0]['name'] = $value[0]['url']; } - if ($imageName = $this->getUploadedImageName($value)) { + if ($imageName = $this->getFinalImageName($value)) { $object->setData($this->additionalData . $attributeName, $value); $object->setData($attributeName, $imageName); } elseif (!is_string($value)) { @@ -113,8 +130,9 @@ public function beforeSave($object) } /** - * @return \Magento\Catalog\Model\ImageUploader + * Get Image Uploader instance * + * @return \Magento\Catalog\Model\ImageUploader * @deprecated 101.0.0 */ private function getImageUploader() diff --git a/app/code/Magento/Catalog/Model/ImageUploader.php b/app/code/Magento/Catalog/Model/ImageUploader.php index ce92a2c1d958d..afa946e9be332 100644 --- 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 */ @@ -192,7 +194,7 @@ public function getFilePath($path, $imageName) * * @param string $imageName * - * @return string + * @return string $destinationImageName saved filename * * @throws \Magento\Framework\Exception\LocalizedException */ @@ -201,7 +203,9 @@ public function moveFileFromTmp($imageName) $baseTmpPath = $this->getBaseTmpPath(); $basePath = $this->getBasePath(); - $baseImagePath = $this->getFilePath($basePath, $imageName); + $destinationImageName = $this->getFinalImageName($imageName); + + $baseImagePath = $this->getFilePath($basePath, $destinationImageName); $baseTmpImagePath = $this->getFilePath($baseTmpPath, $imageName); try { @@ -219,7 +223,7 @@ public function moveFileFromTmp($imageName) ); } - return $imageName; + return $destinationImageName; } /** @@ -276,4 +280,23 @@ public function saveFileToTmpDir($fileId) return $result; } + + /** + * Get the final filename to use when the image is saved + * + * @param string $imageName + * @return string + */ + public function getFinalImageName($imageName) + { + $basePath = $this->getBasePath(); + + /** + * Get the absolute path to the new destination and then retrieve a new filename to prevent overwriting + * existing category images with the same filename + */ + $absolutePath = $this->mediaDirectory->getAbsolutePath($this->getFilePath($basePath, $imageName)); + + return Uploader::getNewFileName($absolutePath); + } } 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 index f1672d842de4e..08a7352e69398 100644 --- 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 @@ -67,7 +67,10 @@ protected function setUp() $this->imageUploader = $this->createPartialMock( \Magento\Catalog\Model\ImageUploader::class, - ['moveFileFromTmp'] + [ + 'moveFileFromTmp', + 'getFinalImageName', + ] ); $this->filesystem = $this->getMockBuilder(\Magento\Framework\Filesystem::class)->disableOriginalConstructor() @@ -146,9 +149,11 @@ public function testBeforeSaveValueInvalid($value) */ public function testBeforeSaveAttributeFileName() { - $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class); + $model = $this->setUpModelForBeforeSave(); $model->setAttribute($this->attribute); + $this->imageUploader->expects($this->any())->method('getFinalImageName')->willReturn('test123.jpg'); + $object = new \Magento\Framework\DataObject([ 'test_attribute' => [ ['name' => 'test123.jpg'] @@ -161,13 +166,40 @@ public function testBeforeSaveAttributeFileName() } /** - * Test beforeSaveAttributeFileNameOutsideOfCategoryDir. + * @return \Magento\Catalog\Model\Category\Attribute\Backend\Image */ - public function testBeforeSaveAttributeFileNameOutsideOfCategoryDir() + private function setUpModelForBeforeSave() { + $objectManagerMock = $this->createPartialMock(\Magento\Framework\App\ObjectManager::class, ['get']); + + $imageUploaderMock = $this->imageUploader; + + $objectManagerMock->expects($this->any()) + ->method('get') + ->will($this->returnCallback(function ($class, $params = []) use ($imageUploaderMock) { + if ($class === "Magento\Catalog\CategoryImageUpload") { + 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); + } + + /** + * Test beforeSaveAttributeFileNameOutsideOfCategoryDir. + */ + public function testBeforeSaveAttributeFileNameOutsideOfCategoryDir() + { + $model = $this->setUpModelForBeforeSave(); $model->setAttribute($this->attribute); @@ -186,6 +218,9 @@ public function testBeforeSaveAttributeFileNameOutsideOfCategoryDir() ] ]); + $this->imageUploader->expects($this->any())->method('getFinalImageName') + ->willReturn('/pub/media/wysiwyg/test123.jpg'); + $model->beforeSave($object); $this->assertEquals('/pub/media/wysiwyg/test123.jpg', $object->getTestAttribute()); @@ -200,7 +235,7 @@ public function testBeforeSaveAttributeFileNameOutsideOfCategoryDir() */ public function testBeforeSaveTemporaryAttribute() { - $model = $this->objectManager->getObject(\Magento\Catalog\Model\Category\Attribute\Backend\Image::class); + $model = $this->setUpModelForBeforeSave(); $model->setAttribute($this->attribute); $object = new \Magento\Framework\DataObject([ @@ -209,6 +244,11 @@ public function testBeforeSaveTemporaryAttribute() ] ]); + $this->imageUploader->expects($this->any())->method('getFinalImageName') + ->willReturn( + ['name' => 'test123.jpg', 'tmp_name' => 'abc123', 'url' => 'http://www.example.com/test123.jpg'] + ); + $model->beforeSave($object); $this->assertEquals([ @@ -246,7 +286,7 @@ private function setUpModelForAfterSave() $objectManagerMock->expects($this->any()) ->method('get') ->will($this->returnCallback(function ($class, $params = []) use ($imageUploaderMock) { - if ($class == \Magento\Catalog\CategoryImageUpload::class) { + if ($class === Magento\Catalog\CategoryImageUpload::class) { return $imageUploaderMock; }