Skip to content

Commit 7928751

Browse files
adifucanadifucan
authored
[AWS S3] MC-37601: Support by Magento ImportExport (#6231)
1 parent 63a5bd2 commit 7928751

File tree

25 files changed

+405
-123
lines changed

25 files changed

+405
-123
lines changed

‎app/code/Magento/AwsS3/Driver/AwsS3.php‎

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use Magento\Framework\Exception\FileSystemException;
1313
use Magento\Framework\Filesystem\DriverInterface;
1414
use Magento\Framework\Phrase;
15+
use Psr\Log\LoggerInterface;
1516

1617
/**
1718
* Driver for AWS S3 IO operations.
@@ -35,23 +36,35 @@ class AwsS3 implements DriverInterface
3536
*/
3637
private $streams = [];
3738

39+
/**
40+
* @var LoggerInterface
41+
*/
42+
private $logger;
43+
3844
/**
3945
* @param AwsS3Adapter $adapter
46+
* @param LoggerInterface $logger
4047
*/
41-
public function __construct(AwsS3Adapter $adapter)
42-
{
48+
public function __construct(
49+
AwsS3Adapter $adapter,
50+
LoggerInterface $logger
51+
) {
4352
$this->adapter = $adapter;
53+
$this->logger = $logger;
4454
}
4555

4656
/**
4757
* Destroy opened streams.
48-
*
49-
* @throws FileSystemException
5058
*/
5159
public function __destruct()
5260
{
53-
foreach ($this->streams as $stream) {
54-
$this->fileClose($stream);
61+
try {
62+
foreach ($this->streams as $stream) {
63+
$this->fileClose($stream);
64+
}
65+
} catch (\Exception $e) {
66+
// log exception as throwing an exception from a destructor causes a fatal error
67+
$this->logger->critical($e);
5568
}
5669
}
5770

@@ -521,12 +534,17 @@ public function fileRead($resource, $length): string
521534
*/
522535
public function fileGetCsv($resource, $length = 0, $delimiter = ',', $enclosure = '"', $escape = '\\')
523536
{
524-
//phpcs:disable
525-
$metadata = stream_get_meta_data($resource);
526-
//phpcs:enable
527-
$file = $this->adapter->read($metadata['uri'])['contents'];
528-
529-
return str_getcsv($file, $delimiter, $enclosure, $escape);
537+
//phpcs:ignore Magento2.Functions.DiscouragedFunction
538+
$result = fgetcsv($resource, $length, $delimiter, $enclosure, $escape);
539+
if ($result === null) {
540+
throw new FileSystemException(
541+
new Phrase(
542+
'The "%1" CSV handle is incorrect. Verify the handle and try again.',
543+
[$this->getWarningMessage()]
544+
)
545+
);
546+
}
547+
return $result;
530548
}
531549

532550
/**

‎app/code/Magento/AwsS3/Test/Unit/Driver/AwsS3Test.php‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use Magento\Framework\Exception\FileSystemException;
1414
use PHPUnit\Framework\MockObject\MockObject;
1515
use PHPUnit\Framework\TestCase;
16+
use Psr\Log\LoggerInterface;
1617

1718
/**
1819
* @see AwsS3
@@ -36,13 +37,19 @@ class AwsS3Test extends TestCase
3637
*/
3738
private $clientMock;
3839

40+
/**
41+
* @var LoggerInterface
42+
*/
43+
private $logger;
44+
3945
/**
4046
* @inheritDoc
4147
*/
4248
protected function setUp(): void
4349
{
4450
$this->adapterMock = $this->createMock(AwsS3Adapter::class);
4551
$this->clientMock = $this->getMockForAbstractClass(S3ClientInterface::class);
52+
$this->logger = $this->getMockForAbstractClass(LoggerInterface::class);
4653

4754
$this->adapterMock->method('applyPathPrefix')
4855
->willReturnArgument(0);
@@ -59,7 +66,7 @@ protected function setUp(): void
5966
return self::URL . $path;
6067
});
6168

62-
$this->driver = new AwsS3($this->adapterMock);
69+
$this->driver = new AwsS3($this->adapterMock, $this->logger);
6370
}
6471

6572
/**
@@ -149,6 +156,16 @@ public function getAbsolutePathDataProvider(): array
149156
'',
150157
self::URL . 'media/catalog/test.png',
151158
self::URL . 'media/catalog/test.png'
159+
],
160+
[
161+
self::URL,
162+
'var/import/images',
163+
self::URL . 'var/import/images'
164+
],
165+
[
166+
self::URL . 'var/import/images/product_images/',
167+
self::URL . 'var/import/images/product_images/1.png',
168+
self::URL . 'var/import/images/product_images/1.png'
152169
]
153170
];
154171
}

‎app/code/Magento/CatalogImportExport/Model/Import/Product.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
use Magento\Catalog\Api\ProductRepositoryInterface;
1010
use Magento\Catalog\Model\Config as CatalogConfig;
1111
use Magento\Catalog\Model\Product\Visibility;
12-
use Magento\Catalog\Model\ResourceModel\Product\Link;
1312
use Magento\CatalogImportExport\Model\Import\Product\ImageTypeProcessor;
1413
use Magento\CatalogImportExport\Model\Import\Product\LinkProcessor;
1514
use Magento\CatalogImportExport\Model\Import\Product\MediaGalleryProcessor;
@@ -2209,6 +2208,11 @@ protected function _getUploader()
22092208
$dirConfig = DirectoryList::getDefaultConfig();
22102209
$dirAddon = $dirConfig[DirectoryList::MEDIA][DirectoryList::PATH];
22112210

2211+
// make media folder a primary folder for media in external storages
2212+
if (!is_a($this->_mediaDirectory->getDriver(), File::class)) {
2213+
$dirAddon = DirectoryList::MEDIA;
2214+
}
2215+
22122216
$tmpPath = $this->getImportDir();
22132217

22142218
if (!$fileUploader->setTmpDir($tmpPath)) {

‎app/code/Magento/CatalogImportExport/Model/Import/Uploader.php‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Magento\Framework\App\ObjectManager;
1010
use Magento\Framework\Exception\ValidatorException;
1111
use Magento\Framework\Filesystem;
12+
use Magento\Framework\Filesystem\Directory\TargetDirectory;
1213
use Magento\Framework\Filesystem\DriverPool;
1314

1415
/**
@@ -116,6 +117,11 @@ class Uploader extends \Magento\MediaStorage\Model\File\Uploader
116117
*/
117118
private $maxFilenameLength = 255;
118119

120+
/**
121+
* @var TargetDirectory
122+
*/
123+
private $targetDirectory;
124+
119125
/**
120126
* @param \Magento\MediaStorage\Helper\File\Storage\Database $coreFileStorageDb
121127
* @param \Magento\MediaStorage\Helper\File\Storage $coreFileStorage
@@ -125,6 +131,7 @@ class Uploader extends \Magento\MediaStorage\Model\File\Uploader
125131
* @param Filesystem\File\ReadFactory $readFactory
126132
* @param string|null $filePath
127133
* @param \Magento\Framework\Math\Random|null $random
134+
* @param TargetDirectory|null $targetDirectory
128135
* @throws \Magento\Framework\Exception\FileSystemException
129136
* @throws \Magento\Framework\Exception\LocalizedException
130137
*/
@@ -136,7 +143,8 @@ public function __construct(
136143
Filesystem $filesystem,
137144
Filesystem\File\ReadFactory $readFactory,
138145
$filePath = null,
139-
\Magento\Framework\Math\Random $random = null
146+
\Magento\Framework\Math\Random $random = null,
147+
TargetDirectory $targetDirectory = null
140148
) {
141149
$this->_imageFactory = $imageFactory;
142150
$this->_coreFileStorageDb = $coreFileStorageDb;
@@ -149,6 +157,7 @@ public function __construct(
149157
$this->_setUploadFile($filePath);
150158
}
151159
$this->random = $random ?: ObjectManager::getInstance()->get(\Magento\Framework\Math\Random::class);
160+
$this->targetDirectory = $targetDirectory ?: ObjectManager::getInstance()->get(TargetDirectory::class);
152161
}
153162

154163
/**
@@ -188,7 +197,8 @@ public function move($fileName, $renameFileOff = false)
188197
}
189198

190199
$this->_setUploadFile($tmpFilePath);
191-
$destDir = $this->_directory->getAbsolutePath($this->getDestDir());
200+
$rootDirectory = $this->getTargetDirectory()->getDirectoryRead(DirectoryList::ROOT);
201+
$destDir = $rootDirectory->getAbsolutePath($this->getDestDir());
192202
$result = $this->save($destDir);
193203
unset($result['path']);
194204
$result['name'] = self::getCorrectFileName($result['name']);
@@ -243,6 +253,20 @@ private function downloadFileFromUrl($url, $driver)
243253
return $tmpFilePath;
244254
}
245255

256+
/**
257+
* Retrieves target directory.
258+
*
259+
* @return TargetDirectory
260+
*/
261+
private function getTargetDirectory(): TargetDirectory
262+
{
263+
if (!isset($this->targetDirectory)) {
264+
$this->targetDirectory = ObjectManager::getInstance()->get(TargetDirectory::class);
265+
}
266+
267+
return $this->targetDirectory;
268+
}
269+
246270
/**
247271
* Prepare information about the file for moving
248272
*
@@ -381,7 +405,8 @@ public function getDestDir()
381405
*/
382406
public function setDestDir($path)
383407
{
384-
if (is_string($path) && $this->_directory->isWritable($path)) {
408+
$directoryRoot = $this->getTargetDirectory()->getDirectoryWrite(DirectoryList::ROOT);
409+
if (is_string($path) && $directoryRoot->isWritable($path)) {
385410
$this->_destDir = $path;
386411
return true;
387412
}
@@ -404,7 +429,8 @@ protected function _moveFile($tmpPath, $destPath)
404429
$destinationRealPath = $this->_directory->getDriver()->getRealPath($destPath);
405430
$relativeDestPath = $this->_directory->getRelativePath($destPath);
406431
$isSameFile = $tmpRealPath === $destinationRealPath;
407-
return $isSameFile ?: $this->_directory->copyFile($tmpPath, $relativeDestPath);
432+
$rootDirectory = $this->getTargetDirectory()->getDirectoryWrite(DirectoryList::ROOT);
433+
return $isSameFile ?: $this->_directory->copyFile($tmpPath, $relativeDestPath, $rootDirectory);
408434
} else {
409435
return false;
410436
}

‎app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/ProductTest.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
use Magento\Framework\Exception\LocalizedException;
4040
use Magento\Framework\Filesystem;
4141
use Magento\Framework\Filesystem\Directory\WriteInterface;
42+
use Magento\Framework\Filesystem\Driver\File as DriverFile;
4243
use Magento\Framework\Indexer\IndexerRegistry;
4344
use Magento\Framework\Json\Helper\Data;
4445
use Magento\Framework\Model\ResourceModel\Db\ObjectRelationProcessor;
@@ -207,6 +208,9 @@ class ProductTest extends AbstractImportTestCase
207208
/** @var ImageTypeProcessor|MockObject */
208209
protected $imageTypeProcessor;
209210

211+
/** @var DriverFile|MockObject */
212+
private $driverFile;
213+
210214
/**
211215
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
212216
*/
@@ -374,6 +378,10 @@ protected function setUp(): void
374378

375379
$this->errorAggregator = $this->getErrorAggregatorObject();
376380

381+
$this->driverFile = $this->getMockBuilder(DriverFile::class)
382+
->disableOriginalConstructor()
383+
->getMock();
384+
377385
$this->data = [];
378386

379387
$this->imageTypeProcessor = $this->getMockBuilder(ImageTypeProcessor::class)
@@ -1336,6 +1344,10 @@ public function testFillUploaderObject($isRead, $isWrite, $message)
13361344
->with('pub/media/catalog/product')
13371345
->willReturn($isWrite);
13381346

1347+
$this->_mediaDirectory
1348+
->method('getDriver')
1349+
->willReturn($this->driverFile);
1350+
13391351
$this->_mediaDirectory
13401352
->method('getRelativePath')
13411353
->willReturnMap(

‎app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/UploaderTest.php‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Magento\CatalogImportExport\Model\Import\Uploader;
1010
use Magento\Framework\Exception\LocalizedException;
1111
use Magento\Framework\Filesystem;
12+
use Magento\Framework\Filesystem\Directory\TargetDirectory;
1213
use Magento\Framework\Filesystem\Directory\Write;
1314
use Magento\Framework\Filesystem\Driver\Http;
1415
use Magento\Framework\Filesystem\Driver\Https;
@@ -73,6 +74,11 @@ class UploaderTest extends TestCase
7374
*/
7475
protected $uploader;
7576

77+
/**
78+
* @var TargetDirectory|MockObject
79+
*/
80+
private $targetDirectory;
81+
7682
protected function setUp(): void
7783
{
7884
$this->coreFileStorageDb = $this->getMockBuilder(Database::class)
@@ -115,6 +121,13 @@ protected function setUp(): void
115121
->setMethods(['getRandomString'])
116122
->getMock();
117123

124+
$this->targetDirectory = $this->getMockBuilder(TargetDirectory::class)
125+
->disableOriginalConstructor()
126+
->setMethods(['getDirectoryWrite', 'getDirectoryRead'])
127+
->getMock();
128+
$this->targetDirectory->method('getDirectoryWrite')->willReturn($this->directoryMock);
129+
$this->targetDirectory->method('getDirectoryRead')->willReturn($this->directoryMock);
130+
118131
$this->uploader = $this->getMockBuilder(Uploader::class)
119132
->setConstructorArgs(
120133
[
@@ -125,7 +138,8 @@ protected function setUp(): void
125138
$this->filesystem,
126139
$this->readFactory,
127140
null,
128-
$this->random
141+
$this->random,
142+
$this->targetDirectory
129143
]
130144
)
131145
->setMethods(['_setUploadFile', 'save', 'getTmpDir', 'checkAllowedExtension'])
@@ -274,9 +288,9 @@ public function testMoveFileUrlDrivePool($fileUrl, $expectedHost, $expectedDrive
274288
->addMethods(['readAll'])
275289
->onlyMethods(['isExists'])
276290
->getMock();
277-
$driverMock->expects($this->any())->method('isExists')->willReturn(true);
278-
$driverMock->expects($this->any())->method('readAll')->willReturn(null);
279-
$driverPool->expects($this->any())->method('getDriver')->willReturn($driverMock);
291+
$driverMock->method('isExists')->willReturn(true);
292+
$driverMock->method('readAll')->willReturn(null);
293+
$driverPool->method('getDriver')->willReturn($driverMock);
280294

281295
$readFactory = $this->getMockBuilder(ReadFactory::class)
282296
->setConstructorArgs(
@@ -287,10 +301,11 @@ public function testMoveFileUrlDrivePool($fileUrl, $expectedHost, $expectedDrive
287301
->setMethods(['create'])
288302
->getMock();
289303

290-
$readFactory->expects($this->any())->method('create')
304+
$readFactory->method('create')
291305
->with($expectedHost, $expectedScheme)
292306
->willReturn($driverMock);
293307

308+
/** @var Uploader $uploaderMock */
294309
$uploaderMock = $this->getMockBuilder(Uploader::class)
295310
->setConstructorArgs(
296311
[
@@ -300,6 +315,9 @@ public function testMoveFileUrlDrivePool($fileUrl, $expectedHost, $expectedDrive
300315
$this->validator,
301316
$this->filesystem,
302317
$readFactory,
318+
null,
319+
$this->random,
320+
$this->targetDirectory
303321
]
304322
)
305323
->getMock();

‎app/code/Magento/DownloadableImportExport/Helper/Uploader.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
namespace Magento\DownloadableImportExport\Helper;
77

88
use Magento\Framework\App\Filesystem\DirectoryList;
9+
use Magento\Framework\Filesystem\Driver\File;
910

1011
/**
1112
* Uploader helper for downloadable products
@@ -82,6 +83,11 @@ public function getUploader($type, $parameters)
8283
$dirConfig = DirectoryList::getDefaultConfig();
8384
$dirAddon = $dirConfig[DirectoryList::MEDIA][DirectoryList::PATH];
8485

86+
// make media folder a primary folder for media in external storages
87+
if (!is_a($this->mediaDirectory->getDriver(), File::class)) {
88+
$dirAddon = DirectoryList::MEDIA;
89+
}
90+
8591
if (!empty($parameters[\Magento\ImportExport\Model\Import::FIELD_NAME_IMG_FILE_DIR])) {
8692
$tmpPath = $parameters[\Magento\ImportExport\Model\Import::FIELD_NAME_IMG_FILE_DIR];
8793
} else {
@@ -113,7 +119,9 @@ public function getUploader($type, $parameters)
113119
*/
114120
public function isFileExist(string $fileName): bool
115121
{
116-
return $this->mediaDirectory->isExist($this->fileUploader->getDestDir().$fileName);
122+
$fileName = '/' . ltrim($fileName, '/');
123+
124+
return $this->mediaDirectory->isExist($this->fileUploader->getDestDir() . $fileName);
117125
}
118126

119127
/**

0 commit comments

Comments
 (0)