Skip to content

Commit bf1f01c

Browse files
arnobsharnobsh
committed
Merge remote-tracking branch 'magento-mpi/MC-22390' into MPI-PR-2019-10-31
2 parents bd2da5b + b4522be commit bf1f01c

File tree

3 files changed

+80
-24
lines changed

3 files changed

+80
-24
lines changed

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

+20-11
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,6 @@ class Uploader extends \Magento\MediaStorage\Model\File\Uploader
3636
*/
3737
protected $_tmpDir = '';
3838

39-
/**
40-
* Download directory for url-based resources.
41-
*
42-
* @var string
43-
*/
44-
private $downloadDir;
45-
4639
/**
4740
* Destination directory.
4841
*
@@ -151,7 +144,6 @@ public function __construct(
151144
$this->_setUploadFile($filePath);
152145
}
153146
$this->random = $random ?: ObjectManager::getInstance()->get(\Magento\Framework\Math\Random::class);
154-
$this->downloadDir = DirectoryList::getDefaultConfig()[DirectoryList::TMP][DirectoryList::PATH];
155147
}
156148

157149
/**
@@ -187,8 +179,7 @@ public function move($fileName, $renameFileOff = false)
187179
$driver = ($matches[0] === $this->httpScheme) ? DriverPool::HTTP : DriverPool::HTTPS;
188180
$tmpFilePath = $this->downloadFileFromUrl($url, $driver);
189181
} else {
190-
$tmpDir = $this->getTmpDir() ? ($this->getTmpDir() . '/') : '';
191-
$tmpFilePath = $this->_directory->getRelativePath($tmpDir . $fileName);
182+
$tmpFilePath = $this->_directory->getRelativePath($this->getTempFilePath($fileName));
192183
}
193184

194185
$this->_setUploadFile($tmpFilePath);
@@ -225,8 +216,13 @@ private function downloadFileFromUrl($url, $driver)
225216
$tmpFileName = str_replace(".$fileExtension", '', $fileName);
226217
$tmpFileName .= '_' . $this->random->getRandomString(16);
227218
$tmpFileName .= $fileExtension ? ".$fileExtension" : '';
228-
$tmpFilePath = $this->_directory->getRelativePath($this->downloadDir . '/' . $tmpFileName);
219+
$tmpFilePath = $this->_directory->getRelativePath($this->getTempFilePath($tmpFileName));
229220

221+
if (!$this->_directory->isWritable($this->getTmpDir())) {
222+
throw new \Magento\Framework\Exception\LocalizedException(
223+
__('Import images directory must be writable in order to process remote images.')
224+
);
225+
}
230226
$this->_directory->writeFile(
231227
$tmpFilePath,
232228
$this->_readFactory->create($url, $driver)->readAll()
@@ -402,6 +398,19 @@ protected function _moveFile($tmpPath, $destPath)
402398
}
403399
}
404400

401+
/**
402+
* Append temp path to filename
403+
*
404+
* @param string $filename
405+
* @return string
406+
*/
407+
private function getTempFilePath(string $filename): string
408+
{
409+
return $this->getTmpDir()
410+
? rtrim($this->getTmpDir(), DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR . $filename
411+
: $filename;
412+
}
413+
405414
/**
406415
* @inheritdoc
407416
*/

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

+6-6
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ public function testMoveFileUrl($fileUrl, $expectedHost, $expectedFileName, $che
128128
{
129129
$tmpDir = 'var/tmp';
130130
$destDir = 'var/dest/dir';
131+
$this->uploader->method('getTmpDir')->willReturn($tmpDir);
131132

132133
// Expected invocation to validate file extension
133134
$this->uploader->expects($this->exactly($checkAllowedExtension))->method('checkAllowedExtension')
@@ -159,9 +160,11 @@ public function testMoveFileUrl($fileUrl, $expectedHost, $expectedFileName, $che
159160
$this->directoryMock->expects($this->any())->method('writeFile')
160161
->will($this->returnValue($expectedFileName));
161162

162-
// Expected invocations to move the temp file to the destination directory
163-
$this->directoryMock->expects($this->once())->method('isWritable')
164-
->with($destDir)
163+
// Expected invocations save the downloaded file to temp file
164+
// and move the temp file to the destination directory
165+
$this->directoryMock->expects($this->exactly(2))
166+
->method('isWritable')
167+
->withConsecutive([$destDir], [$tmpDir])
165168
->willReturn(true);
166169
$this->directoryMock->expects($this->once())->method('getAbsolutePath')
167170
->with($destDir)
@@ -172,9 +175,6 @@ public function testMoveFileUrl($fileUrl, $expectedHost, $expectedFileName, $che
172175
->with($destDir . '/' . $expectedFileName)
173176
->willReturn(['name' => $expectedFileName, 'path' => 'absPath']);
174177

175-
// Do not use configured temp directory
176-
$this->uploader->expects($this->never())->method('getTmpDir');
177-
178178
$this->uploader->setDestDir($destDir);
179179
$result = $this->uploader->move($fileUrl);
180180

‎dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/UploaderTest.php

+54-7
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@
1616
*/
1717
class UploaderTest extends \Magento\TestFramework\Indexer\TestCase
1818
{
19+
/**
20+
* Random string appended to downloaded image name
21+
*/
22+
const RANDOM_STRING = 'BRV8TAuR2AT88OH0';
1923
/**
2024
* @var \Magento\Framework\ObjectManagerInterface
2125
*/
@@ -30,14 +34,29 @@ class UploaderTest extends \Magento\TestFramework\Indexer\TestCase
3034
* @var \Magento\CatalogImportExport\Model\Import\Uploader
3135
*/
3236
private $uploader;
37+
/**
38+
* @var \Magento\Framework\Filesystem\File\ReadInterface|\PHPUnit\Framework\MockObject\MockObject
39+
*/
40+
private $fileReader;
3341

3442
/**
3543
* @inheritdoc
3644
*/
3745
protected function setUp()
3846
{
3947
$this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
40-
$this->uploader = $this->objectManager->create(\Magento\CatalogImportExport\Model\Import\Uploader::class);
48+
$this->fileReader = $this->getMockForAbstractClass(\Magento\Framework\Filesystem\File\ReadInterface::class);
49+
$fileReadFactory = $this->createMock(\Magento\Framework\Filesystem\File\ReadFactory::class);
50+
$fileReadFactory->method('create')->willReturn($this->fileReader);
51+
$random = $this->createMock(\Magento\Framework\Math\Random::class);
52+
$random->method('getRandomString')->willReturn(self::RANDOM_STRING);
53+
$this->uploader = $this->objectManager->create(
54+
\Magento\CatalogImportExport\Model\Import\Uploader::class,
55+
[
56+
'random' => $random,
57+
'readFactory' => $fileReadFactory
58+
]
59+
);
4160

4261
$filesystem = $this->objectManager->create(\Magento\Framework\Filesystem::class);
4362

@@ -60,16 +79,32 @@ protected function setUp()
6079
parent::setUp();
6180
}
6281

82+
/**
83+
* Tests move with external url
84+
*
85+
* @magentoAppIsolation enabled
86+
* @return void
87+
*/
88+
public function testMoveWithExternalURL(): void
89+
{
90+
$fileName = 'http://magento.com/static/images/random_image.jpg';
91+
$this->fileReader->method('readAll')->willReturn(file_get_contents($this->getTestImagePath()));
92+
$this->uploader->move($fileName);
93+
$destFilePath = $this->uploader->getTmpDir() . '/' . 'random_image_' . self::RANDOM_STRING . '.jpg';
94+
$this->assertTrue($this->directory->isExist($destFilePath));
95+
}
96+
6397
/**
6498
* @magentoAppIsolation enabled
6599
* @return void
66100
*/
67101
public function testMoveWithValidFile(): void
68102
{
69-
$fileName = 'magento_additional_image_one.jpg';
103+
$testImagePath = $this->getTestImagePath();
104+
$fileName = basename($testImagePath);
70105
$filePath = $this->directory->getAbsolutePath($this->uploader->getTmpDir() . '/' . $fileName);
71106
//phpcs:ignore
72-
copy(__DIR__ . '/_files/' . $fileName, $filePath);
107+
copy($testImagePath, $filePath);
73108
$this->uploader->move($fileName);
74109
$this->assertTrue($this->directory->isExist($this->uploader->getTmpDir() . '/' . $fileName));
75110
}
@@ -84,15 +119,17 @@ public function testMoveWithValidFile(): void
84119
public function testMoveWithFileOutsideTemp(): void
85120
{
86121
$tmpDir = $this->uploader->getTmpDir();
87-
if (!$this->directory->create($newTmpDir = $tmpDir .'/test1')) {
122+
$newTmpDir = $tmpDir . '/test1';
123+
if (!$this->directory->create($newTmpDir)) {
88124
throw new \RuntimeException('Failed to create temp dir');
89125
}
90126
$this->uploader->setTmpDir($newTmpDir);
91-
$fileName = 'magento_additional_image_one.jpg';
127+
$testImagePath = $this->getTestImagePath();
128+
$fileName = basename($testImagePath);
92129
$filePath = $this->directory->getAbsolutePath($tmpDir . '/' . $fileName);
93130
//phpcs:ignore
94-
copy(__DIR__ . '/_files/' . $fileName, $filePath);
95-
$this->uploader->move('../' .$fileName);
131+
copy($testImagePath, $filePath);
132+
$this->uploader->move('../' . $fileName);
96133
$this->assertTrue($this->directory->isExist($tmpDir . '/' . $fileName));
97134
}
98135

@@ -111,4 +148,14 @@ public function testMoveWithInvalidFile(): void
111148
$this->uploader->move($fileName);
112149
$this->assertFalse($this->directory->isExist($this->uploader->getTmpDir() . '/' . $fileName));
113150
}
151+
152+
/**
153+
* Get the full path to the test image
154+
*
155+
* @return string
156+
*/
157+
private function getTestImagePath(): string
158+
{
159+
return __DIR__ . '/_files/magento_additional_image_one.jpg';
160+
}
114161
}

0 commit comments

Comments
 (0)