Skip to content

Commit 5192283

Browse files
gabrieldagamagabrieldagama
authored
ENGCOM-8031: Refactor moveFileFromTmp to not use a static method call #27581 
2 parents 49a9b84 + 810b36a commit 5192283

File tree

5 files changed

+177
-51
lines changed

5 files changed

+177
-51
lines changed

‎app/code/Magento/Catalog/Model/ImageUploader.php

Lines changed: 44 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -5,114 +5,116 @@
55
*/
66
namespace Magento\Catalog\Model;
77

8+
use Magento\Framework\App\Filesystem\DirectoryList;
9+
use Magento\Framework\Exception\LocalizedException;
810
use Magento\Framework\File\Uploader;
11+
use Magento\Framework\App\ObjectManager;
12+
use Magento\Framework\File\Name;
13+
use Magento\Framework\Filesystem;
14+
use Magento\Framework\Filesystem\Directory\WriteInterface;
15+
use Magento\MediaStorage\Helper\File\Storage\Database;
16+
use Magento\MediaStorage\Model\File\UploaderFactory;
17+
use Magento\Store\Model\StoreManagerInterface;
18+
use Psr\Log\LoggerInterface;
919

1020
/**
1121
* Catalog image uploader
1222
*/
1323
class ImageUploader
1424
{
1525
/**
16-
* Core file storage database
17-
*
18-
* @var \Magento\MediaStorage\Helper\File\Storage\Database
26+
* @var Database
1927
*/
2028
protected $coreFileStorageDatabase;
2129

2230
/**
23-
* Media directory object (writable).
24-
*
25-
* @var \Magento\Framework\Filesystem\Directory\WriteInterface
31+
* @var WriteInterface
2632
*/
2733
protected $mediaDirectory;
2834

2935
/**
30-
* Uploader factory
31-
*
32-
* @var \Magento\MediaStorage\Model\File\UploaderFactory
36+
* @var UploaderFactory
3337
*/
3438
private $uploaderFactory;
3539

3640
/**
37-
* Store manager
38-
*
39-
* @var \Magento\Store\Model\StoreManagerInterface
41+
* @var StoreManagerInterface
4042
*/
4143
protected $storeManager;
4244

4345
/**
44-
* @var \Psr\Log\LoggerInterface
46+
* @var LoggerInterface
4547
*/
4648
protected $logger;
4749

4850
/**
49-
* Base tmp path
50-
*
5151
* @var string
5252
*/
5353
protected $baseTmpPath;
5454

5555
/**
56-
* Base path
57-
*
5856
* @var string
5957
*/
6058
protected $basePath;
6159

6260
/**
63-
* Allowed extensions
64-
*
6561
* @var string
6662
*/
6763
protected $allowedExtensions;
6864

6965
/**
70-
* List of allowed image mime types
71-
*
7266
* @var string[]
7367
*/
7468
private $allowedMimeTypes;
7569

7670
/**
77-
* ImageUploader constructor
71+
* @var Name
72+
*/
73+
private $fileNameLookup;
74+
75+
/**
76+
* ImageUploader constructor.
7877
*
79-
* @param \Magento\MediaStorage\Helper\File\Storage\Database $coreFileStorageDatabase
80-
* @param \Magento\Framework\Filesystem $filesystem
81-
* @param \Magento\MediaStorage\Model\File\UploaderFactory $uploaderFactory
82-
* @param \Magento\Store\Model\StoreManagerInterface $storeManager
83-
* @param \Psr\Log\LoggerInterface $logger
78+
* @param Database $coreFileStorageDatabase
79+
* @param Filesystem $filesystem
80+
* @param UploaderFactory $uploaderFactory
81+
* @param StoreManagerInterface $storeManager
82+
* @param LoggerInterface $logger
8483
* @param string $baseTmpPath
8584
* @param string $basePath
8685
* @param string[] $allowedExtensions
8786
* @param string[] $allowedMimeTypes
87+
* @param Name|null $fileNameLookup
88+
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
8889
*/
8990
public function __construct(
90-
\Magento\MediaStorage\Helper\File\Storage\Database $coreFileStorageDatabase,
91-
\Magento\Framework\Filesystem $filesystem,
92-
\Magento\MediaStorage\Model\File\UploaderFactory $uploaderFactory,
93-
\Magento\Store\Model\StoreManagerInterface $storeManager,
94-
\Psr\Log\LoggerInterface $logger,
91+
Database $coreFileStorageDatabase,
92+
Filesystem $filesystem,
93+
UploaderFactory $uploaderFactory,
94+
StoreManagerInterface $storeManager,
95+
LoggerInterface $logger,
9596
$baseTmpPath,
9697
$basePath,
9798
$allowedExtensions,
98-
$allowedMimeTypes = []
99+
$allowedMimeTypes = [],
100+
Name $fileNameLookup = null
99101
) {
100102
$this->coreFileStorageDatabase = $coreFileStorageDatabase;
101-
$this->mediaDirectory = $filesystem->getDirectoryWrite(\Magento\Framework\App\Filesystem\DirectoryList::MEDIA);
103+
$this->mediaDirectory = $filesystem->getDirectoryWrite(DirectoryList::MEDIA);
102104
$this->uploaderFactory = $uploaderFactory;
103105
$this->storeManager = $storeManager;
104106
$this->logger = $logger;
105107
$this->baseTmpPath = $baseTmpPath;
106108
$this->basePath = $basePath;
107109
$this->allowedExtensions = $allowedExtensions;
108110
$this->allowedMimeTypes = $allowedMimeTypes;
111+
$this->fileNameLookup = $fileNameLookup ?? ObjectManager::getInstance()->get(Name::class);
109112
}
110113

111114
/**
112115
* Set base tmp path
113116
*
114117
* @param string $baseTmpPath
115-
*
116118
* @return void
117119
*/
118120
public function setBaseTmpPath($baseTmpPath)
@@ -124,7 +126,6 @@ public function setBaseTmpPath($baseTmpPath)
124126
* Set base path
125127
*
126128
* @param string $basePath
127-
*
128129
* @return void
129130
*/
130131
public function setBasePath($basePath)
@@ -136,7 +137,6 @@ public function setBasePath($basePath)
136137
* Set allowed extensions
137138
*
138139
* @param string[] $allowedExtensions
139-
*
140140
* @return void
141141
*/
142142
public function setAllowedExtensions($allowedExtensions)
@@ -179,7 +179,6 @@ public function getAllowedExtensions()
179179
*
180180
* @param string $path
181181
* @param string $imageName
182-
*
183182
* @return string
184183
*/
185184
public function getFilePath($path, $imageName)
@@ -194,7 +193,7 @@ public function getFilePath($path, $imageName)
194193
* @param bool $returnRelativePath
195194
* @return string
196195
*
197-
* @throws \Magento\Framework\Exception\LocalizedException
196+
* @throws LocalizedException
198197
*/
199198
public function moveFileFromTmp($imageName, $returnRelativePath = false)
200199
{
@@ -203,7 +202,7 @@ public function moveFileFromTmp($imageName, $returnRelativePath = false)
203202

204203
$baseImagePath = $this->getFilePath(
205204
$basePath,
206-
Uploader::getNewFileName(
205+
$this->fileNameLookup->getNewFileName(
207206
$this->mediaDirectory->getAbsolutePath(
208207
$this->getFilePath($basePath, $imageName)
209208
)
@@ -222,10 +221,7 @@ public function moveFileFromTmp($imageName, $returnRelativePath = false)
222221
);
223222
} catch (\Exception $e) {
224223
$this->logger->critical($e);
225-
throw new \Magento\Framework\Exception\LocalizedException(
226-
__('Something went wrong while saving the file(s).'),
227-
$e
228-
);
224+
throw new LocalizedException(__('Something went wrong while saving the file(s).'), $e);
229225
}
230226

231227
return $returnRelativePath ? $baseImagePath : $imageName;
@@ -235,10 +231,9 @@ public function moveFileFromTmp($imageName, $returnRelativePath = false)
235231
* Checking file for save and save it to tmp dir
236232
*
237233
* @param string $fileId
238-
*
239234
* @return string[]
240235
*
241-
* @throws \Magento\Framework\Exception\LocalizedException
236+
* @throws LocalizedException
242237
*/
243238
public function saveFileToTmpDir($fileId)
244239
{
@@ -249,15 +244,13 @@ public function saveFileToTmpDir($fileId)
249244
$uploader->setAllowedExtensions($this->getAllowedExtensions());
250245
$uploader->setAllowRenameFiles(true);
251246
if (!$uploader->checkMimeType($this->allowedMimeTypes)) {
252-
throw new \Magento\Framework\Exception\LocalizedException(__('File validation failed.'));
247+
throw new LocalizedException(__('File validation failed.'));
253248
}
254249
$result = $uploader->save($this->mediaDirectory->getAbsolutePath($baseTmpPath));
255250
unset($result['path']);
256251

257252
if (!$result) {
258-
throw new \Magento\Framework\Exception\LocalizedException(
259-
__('File can not be saved to the destination folder.')
260-
);
253+
throw new LocalizedException(__('File can not be saved to the destination folder.'));
261254
}
262255

263256
/**
@@ -277,7 +270,7 @@ public function saveFileToTmpDir($fileId)
277270
$this->coreFileStorageDatabase->saveFile($relativePath);
278271
} catch (\Exception $e) {
279272
$this->logger->critical($e);
280-
throw new \Magento\Framework\Exception\LocalizedException(
273+
throw new LocalizedException(
281274
__('Something went wrong while saving the file(s).'),
282275
$e
283276
);

‎lib/internal/Magento/Framework/File/Name.php

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Framework\File;
9+
10+
use Magento\Framework\App\Filesystem\DirectoryList;
11+
use Magento\Framework\Exception\FileSystemException;
12+
use Magento\Framework\Validation\ValidationException;
13+
14+
/**
15+
* Utility for generating a unique file name
16+
*/
17+
class Name
18+
{
19+
/**
20+
* Get new file name if the given name is in use
21+
*
22+
* @param string $destinationFile
23+
* @return string
24+
*/
25+
public function getNewFileName(string $destinationFile)
26+
{
27+
$fileInfo = $this->getPathInfo($destinationFile);
28+
if ($this->fileExist($destinationFile)) {
29+
$index = 1;
30+
$baseName = $fileInfo['filename'] . '.' . $fileInfo['extension'];
31+
while ($this->fileExist($fileInfo['dirname'] . '/' . $baseName)) {
32+
$baseName = $fileInfo['filename'] . '_' . $index . '.' . $fileInfo['extension'];
33+
$index++;
34+
}
35+
$destFileName = $baseName;
36+
} else {
37+
return $fileInfo['basename'];
38+
}
39+
40+
return $destFileName;
41+
}
42+
43+
/**
44+
* Get the path information from a given file
45+
*
46+
* @param string $destinationFile
47+
* @return string|string[]
48+
*/
49+
private function getPathInfo(string $destinationFile)
50+
{
51+
return pathinfo($destinationFile);
52+
}
53+
54+
/**
55+
* Check to see if a given file exists
56+
*
57+
* @param string $destinationFile
58+
* @return bool
59+
*/
60+
private function fileExist(string $destinationFile)
61+
{
62+
return file_exists($destinationFile);
63+
}
64+
}

‎lib/internal/Magento/Framework/Test/Unit/File/NameTest.php

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\Framework\Test\Unit\File;
8+
9+
use Magento\Framework\File\Name;
10+
11+
class NameTest extends \PHPUnit\Framework\TestCase
12+
{
13+
/**
14+
* @var string
15+
*/
16+
private $existingFilePath;
17+
18+
/**
19+
* @var string
20+
*/
21+
private $nonExistingFilePath;
22+
23+
/**
24+
* @var string
25+
*/
26+
private $multipleExistingFilePath;
27+
28+
/**
29+
* @var Name
30+
*/
31+
private $name;
32+
33+
/**
34+
* @inheritDoc
35+
*/
36+
protected function setUp(): void
37+
{
38+
$this->name = new Name();
39+
$this->existingFilePath = __DIR__ . '/../_files/source.txt';
40+
$this->multipleExistingFilePath = __DIR__ . '/../_files/name.txt';
41+
$this->nonExistingFilePath = __DIR__ . '/../_files/file.txt';
42+
}
43+
44+
/**
45+
* @test
46+
*/
47+
public function testGetNewFileNameWhenOneFileExists()
48+
{
49+
$this->assertEquals('source_1.txt', $this->name->getNewFileName($this->existingFilePath));
50+
}
51+
52+
/**
53+
* @test
54+
*/
55+
public function testGetNewFileNameWhenTwoFileExists()
56+
{
57+
$this->assertEquals('name_2.txt', $this->name->getNewFileName($this->multipleExistingFilePath));
58+
}
59+
60+
/**
61+
* @test
62+
*/
63+
public function testGetNewFileNameWhenFileDoesNotExist()
64+
{
65+
$this->assertEquals('file.txt', $this->name->getNewFileName($this->nonExistingFilePath));
66+
}
67+
}

‎lib/internal/Magento/Framework/Test/Unit/_files/name.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
name

‎lib/internal/Magento/Framework/Test/Unit/_files/name_1.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
name_1

0 commit comments

Comments
 (0)