Prevent category image filename collisions

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()

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);
+    }
 }

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;
                 }