Skip to content

Commit f165007

Browse files
authored
Merge pull request #5628 from magento-tsg/2.4.0-develop-pr22
[TSG] Fixes for 2.4 (pr22) (2.4.0-develop)
2 parents 58844c0 + 96a549f commit f165007

File tree

13 files changed

+244
-264
lines changed

13 files changed

+244
-264
lines changed

app/code/Magento/Checkout/Test/Unit/Controller/Sidebar/RemoveItemTest.php

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -231,17 +231,20 @@ public function testExecuteWithDbException(): void
231231
$itemId = 1;
232232
$dbError = 'Error';
233233
$message = __('An unspecified error occurred. Please contact us for assistance.');
234-
$response = [
234+
$responseData = [
235235
'success' => false,
236236
'error_message' => $message,
237237
];
238238

239-
$this->getPropertyValue($this->removeItem, 'formKeyValidator')
239+
$this->formKeyValidatorMock
240240
->expects($this->once())
241241
->method('validate')
242242
->with($this->requestMock)
243243
->willReturn(true);
244-
$this->requestMock->expects($this->once())->method('getParam')->with('item_id')->willReturn($itemId);
244+
$this->requestMock->expects($this->once())
245+
->method('getParam')
246+
->with('item_id')
247+
->willReturn($itemId);
245248

246249
$exception = new \Zend_Db_Exception($dbError);
247250

@@ -252,19 +255,21 @@ public function testExecuteWithDbException(): void
252255

253256
$this->loggerMock->expects($this->once())->method('critical')->with($exception);
254257

255-
$this->sidebarMock->expects($this->once())->method('getResponseData')->with($message)->willReturn($response);
256-
$encodedResponse = json_encode($response);
257-
$this->jsonHelperMock->expects($this->once())
258-
->method('jsonEncode')
259-
->with($response)
260-
->willReturn($encodedResponse);
258+
$this->sidebarMock->expects($this->once())
259+
->method('getResponseData')
260+
->with($message)
261+
->willReturn($responseData);
261262

262-
$this->responseMock->expects($this->once())
263-
->method('representJson')
264-
->with($encodedResponse)
265-
->willReturn($this->responseMock);
263+
$resultJson = $this->createMock(ResultJson::class);
264+
$resultJson->expects($this->once())
265+
->method('setData')
266+
->with($responseData)
267+
->willReturnSelf();
268+
$this->resultJsonFactoryMock->expects($this->once())
269+
->method('create')
270+
->willReturn($resultJson);
266271

267-
$this->removeItem->execute();
272+
$this->action->execute();
268273
}
269274

270275
public function testExecuteWhenFormKeyValidationFailed()

app/code/Magento/Eav/Model/Attribute/Data/File.php

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@
77

88
use Magento\Framework\App\Filesystem\DirectoryList;
99
use Magento\Framework\App\RequestInterface;
10+
use Magento\Framework\Filesystem\Io\File as FileIo;
1011

1112
/**
1213
* EAV Entity Attribute File Data Model
1314
*
1415
* @api
1516
* @since 100.0.2
17+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
1618
*/
1719
class File extends \Magento\Eav\Model\Attribute\Data\AbstractData
1820
{
@@ -38,13 +40,19 @@ class File extends \Magento\Eav\Model\Attribute\Data\AbstractData
3840
*/
3941
protected $_directory;
4042

43+
/**
44+
* @var FileIo
45+
*/
46+
private $fileIo;
47+
4148
/**
4249
* @param \Magento\Framework\Stdlib\DateTime\TimezoneInterface $localeDate
4350
* @param \Psr\Log\LoggerInterface $logger
4451
* @param \Magento\Framework\Locale\ResolverInterface $localeResolver
4552
* @param \Magento\Framework\Url\EncoderInterface $urlEncoder
4653
* @param \Magento\MediaStorage\Model\File\Validator\NotProtectedExtension $fileValidator
4754
* @param \Magento\Framework\Filesystem $filesystem
55+
* @param FileIo $fileIo
4856
* @codeCoverageIgnore
4957
*/
5058
public function __construct(
@@ -53,12 +61,14 @@ public function __construct(
5361
\Magento\Framework\Locale\ResolverInterface $localeResolver,
5462
\Magento\Framework\Url\EncoderInterface $urlEncoder,
5563
\Magento\MediaStorage\Model\File\Validator\NotProtectedExtension $fileValidator,
56-
\Magento\Framework\Filesystem $filesystem
64+
\Magento\Framework\Filesystem $filesystem,
65+
FileIo $fileIo
5766
) {
5867
parent::__construct($localeDate, $logger, $localeResolver);
5968
$this->urlEncoder = $urlEncoder;
6069
$this->_fileValidator = $fileValidator;
6170
$this->_directory = $filesystem->getDirectoryWrite(DirectoryList::MEDIA);
71+
$this->fileIo = $fileIo;
6272
}
6373

6474
/**
@@ -86,7 +96,7 @@ public function extractValue(RequestInterface $request)
8696
$mainScope = $this->_requestScope;
8797
$scopes = [];
8898
}
89-
99+
// phpcs:disable Magento2.Security.Superglobal
90100
if (!empty($_FILES[$mainScope])) {
91101
foreach ($_FILES[$mainScope] as $fileKey => $scopeData) {
92102
foreach ($scopes as $scopeName) {
@@ -104,12 +114,15 @@ public function extractValue(RequestInterface $request)
104114
} else {
105115
$value = [];
106116
}
117+
// phpcs:enable Magento2.Security.Superglobal
107118
} else {
119+
// phpcs:disable Magento2.Security.Superglobal
108120
if (isset($_FILES[$attrCode])) {
109121
$value = $_FILES[$attrCode];
110122
} else {
111123
$value = [];
112124
}
125+
// phpcs:enable Magento2.Security.Superglobal
113126
}
114127

115128
if (!empty($extend['delete'])) {
@@ -129,7 +142,7 @@ protected function _validateByRules($value)
129142
{
130143
$label = $this->getAttribute()->getStoreLabel();
131144
$rules = $this->getAttribute()->getValidateRules();
132-
$extension = pathinfo($value['name'], PATHINFO_EXTENSION);
145+
$extension = $this->fileIo->getPathInfo($value['name'])['extension'];
133146

134147
if (!empty($rules['file_extensions'])) {
135148
$extensions = explode(',', $rules['file_extensions']);
@@ -146,7 +159,9 @@ protected function _validateByRules($value)
146159
return $this->_fileValidator->getMessages();
147160
}
148161

149-
if (!empty($value['tmp_name']) && !file_exists($value['tmp_name'])) {
162+
if (!empty($value['tmp_name'])
163+
&& !$this->_directory->getDriver()->isExists($value['tmp_name'])
164+
) {
150165
return [__('"%1" is not a valid file.', $label)];
151166
}
152167

@@ -177,8 +192,9 @@ public function validateValue($value)
177192

178193
if (is_string($value) && !empty($value)) {
179194
$dir = $this->_directory->getAbsolutePath($this->getAttribute()->getEntityType()->getEntityTypeCode());
195+
$stat = $this->_directory->getDriver()->stat($dir . $value);
180196
$fileData = [
181-
'size' => filesize($dir . $value),
197+
'size' => $stat['size'],
182198
'name' => $value,
183199
'tmp_name' => $dir . $value
184200
];
@@ -209,8 +225,6 @@ public function validateValue($value)
209225

210226
if (count($errors) == 0) {
211227
return true;
212-
} elseif (is_string($value) && !empty($value)) {
213-
$this->_directory->delete($dir . $value);
214228
}
215229

216230
return $errors;

app/code/Magento/Eav/Test/Unit/Model/Attribute/Data/FileTest.php

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,42 +6,65 @@
66

77
namespace Magento\Eav\Test\Unit\Model\Attribute\Data;
88

9+
use Magento\Eav\Model\Attribute;
10+
use Magento\Eav\Model\Attribute\Data\File;
11+
use Magento\Eav\Model\AttributeDataFactory;
12+
use Magento\Framework\Filesystem;
13+
use Magento\Framework\Filesystem\Io\File as FileIo;
14+
use Magento\Framework\Locale\ResolverInterface;
15+
use Magento\Framework\Model\AbstractModel;
16+
use Magento\Framework\Stdlib\DateTime\TimezoneInterface;
17+
use Magento\Framework\Url\EncoderInterface;
18+
use Magento\MediaStorage\Model\File\Validator\NotProtectedExtension;
19+
use Psr\Log\LoggerInterface;
20+
use PHPUnit\Framework\MockObject\MockObject;
21+
22+
/**
23+
* Test for Magento\Eav\Model\Attribute\Data\File class.
24+
*
25+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
26+
*/
927
class FileTest extends \PHPUnit\Framework\TestCase
1028
{
1129
/**
12-
* @var \Magento\Eav\Model\Attribute\Data\File
30+
* @var File
1331
*/
1432
protected $model;
1533

1634
/**
17-
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Url\EncoderInterface
35+
* @var MockObject|EncoderInterface
1836
*/
1937
protected $urlEncoder;
2038

2139
/**
22-
* @var \PHPUnit_Framework_MockObject_MockObject
40+
* @var MockObject
2341
*/
2442
protected $fileValidatorMock;
2543

44+
/**
45+
* @inheritDoc
46+
*/
2647
protected function setUp()
2748
{
28-
$timezoneMock = $this->createMock(\Magento\Framework\Stdlib\DateTime\TimezoneInterface::class);
29-
$loggerMock = $this->createMock(\Psr\Log\LoggerInterface::class);
30-
$localeResolverMock = $this->createMock(\Magento\Framework\Locale\ResolverInterface::class);
31-
$this->urlEncoder = $this->createMock(\Magento\Framework\Url\EncoderInterface::class);
49+
$timezoneMock = $this->createMock(TimezoneInterface::class);
50+
$loggerMock = $this->createMock(LoggerInterface::class);
51+
$localeResolverMock = $this->createMock(ResolverInterface::class);
52+
$this->urlEncoder = $this->createMock(EncoderInterface::class);
3253
$this->fileValidatorMock = $this->createPartialMock(
33-
\Magento\MediaStorage\Model\File\Validator\NotProtectedExtension::class,
54+
NotProtectedExtension::class,
3455
['isValid', 'getMessages']
3556
);
36-
$filesystemMock = $this->createMock(\Magento\Framework\Filesystem::class);
57+
$filesystemMock = $this->createMock(Filesystem::class);
58+
$fileIo = $this->createMock(FileIo::class);
3759

38-
$this->model = new \Magento\Eav\Model\Attribute\Data\File(
60+
$this->model = new File(
3961
$timezoneMock,
4062
$loggerMock,
4163
$localeResolverMock,
4264
$this->urlEncoder,
4365
$this->fileValidatorMock,
44-
$filesystemMock
66+
$filesystemMock,
67+
$fileIo
4568
);
4669
}
4770

@@ -56,10 +79,10 @@ protected function setUp()
5679
*/
5780
public function testOutputValue($format, $value, $callTimes, $expectedResult)
5881
{
59-
$entityMock = $this->createMock(\Magento\Framework\Model\AbstractModel::class);
82+
$entityMock = $this->createMock(AbstractModel::class);
6083
$entityMock->expects($this->once())->method('getData')->will($this->returnValue($value));
6184

62-
$attributeMock = $this->createMock(\Magento\Eav\Model\Attribute::class);
85+
$attributeMock = $this->createMock(Attribute::class);
6386
$this->urlEncoder->expects($this->exactly($callTimes))
6487
->method('encode')
6588
->will($this->returnValue('url_key'));
@@ -76,19 +99,19 @@ public function outputValueDataProvider()
7699
{
77100
return [
78101
[
79-
'format' => \Magento\Eav\Model\AttributeDataFactory::OUTPUT_FORMAT_JSON,
102+
'format' => AttributeDataFactory::OUTPUT_FORMAT_JSON,
80103
'value' => 'value',
81104
'callTimes' => 1,
82105
'expectedResult' => ['value' => 'value', 'url_key' => 'url_key'],
83106
],
84107
[
85-
'format' => \Magento\Eav\Model\AttributeDataFactory::OUTPUT_FORMAT_TEXT,
108+
'format' => AttributeDataFactory::OUTPUT_FORMAT_TEXT,
86109
'value' => 'value',
87110
'callTimes' => 0,
88111
'expectedResult' => ''
89112
],
90113
[
91-
'format' => \Magento\Eav\Model\AttributeDataFactory::OUTPUT_FORMAT_TEXT,
114+
'format' => AttributeDataFactory::OUTPUT_FORMAT_TEXT,
92115
'value' => false,
93116
'callTimes' => 0,
94117
'expectedResult' => ''
@@ -119,10 +142,10 @@ public function testValidateValue(
119142
$expectedResult
120143
) {
121144
$this->markTestSkipped('MAGETWO-34751: Test fails after being moved. Might have hidden dependency.');
122-
$entityMock = $this->createMock(\Magento\Framework\Model\AbstractModel::class);
145+
$entityMock = $this->createMock(AbstractModel::class);
123146
$entityMock->expects($this->any())->method('getData')->will($this->returnValue($originalValue));
124147

125-
$attributeMock = $this->createMock(\Magento\Eav\Model\Attribute::class);
148+
$attributeMock = $this->createMock(Attribute::class);
126149
$attributeMock->expects($this->any())->method('getStoreLabel')->will($this->returnValue('Label'));
127150
$attributeMock->expects($this->any())->method('getIsRequired')->will($this->returnValue($isRequired));
128151
$attributeMock->expects($this->any())->method('getIsAjaxRequest')->will($this->returnValue($isAjaxRequest));

app/code/Magento/ImportExport/Controller/Adminhtml/Export/File/Delete.php

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@
1111
use Magento\Framework\App\Action\HttpPostActionInterface;
1212
use Magento\Framework\Exception\FileSystemException;
1313
use Magento\Framework\App\Filesystem\DirectoryList;
14+
use Magento\Framework\Exception\ValidatorException;
1415
use Magento\ImportExport\Controller\Adminhtml\Export as ExportController;
1516
use Magento\Framework\Filesystem;
16-
use Magento\Framework\Filesystem\DriverInterface;
17+
use Magento\Framework\Controller\ResultInterface;
18+
use Magento\Framework\Filesystem\Directory\WriteFactory;
1719

1820
/**
1921
* Controller that delete file by name.
@@ -31,54 +33,58 @@ class Delete extends ExportController implements HttpPostActionInterface
3133
private $filesystem;
3234

3335
/**
34-
* @var DriverInterface
36+
* @var WriteFactory
3537
*/
36-
private $file;
38+
private $writeFactory;
3739

3840
/**
3941
* Delete constructor.
42+
*
4043
* @param Action\Context $context
4144
* @param Filesystem $filesystem
42-
* @param DriverInterface $file
45+
* @param WriteFactory $writeFactory
4346
*/
4447
public function __construct(
4548
Action\Context $context,
4649
Filesystem $filesystem,
47-
DriverInterface $file
50+
WriteFactory $writeFactory
4851
) {
4952
$this->filesystem = $filesystem;
50-
$this->file = $file;
53+
$this->writeFactory = $writeFactory;
5154
parent::__construct($context);
5255
}
5356

5457
/**
5558
* Controller basic method implementation.
5659
*
57-
* @return \Magento\Framework\Controller\ResultInterface
60+
* @return ResultInterface
5861
*/
5962
public function execute()
6063
{
6164
/** @var \Magento\Framework\Controller\Result\Redirect $resultRedirect */
6265
$resultRedirect = $this->resultRedirectFactory->create();
6366
$resultRedirect->setPath('adminhtml/export/index');
64-
$fileName = $this->getRequest()->getParam('filename');
65-
if (empty($fileName) || preg_match('/\.\.(\\\|\/)/', $fileName) !== 0) {
66-
$this->messageManager->addErrorMessage(__('Please provide valid export file name'));
67-
68-
return $resultRedirect;
69-
}
7067
try {
71-
$directory = $this->filesystem->getDirectoryRead(DirectoryList::VAR_DIR);
72-
$path = $directory->getAbsolutePath() . 'export/' . $fileName;
68+
if (empty($fileName = $this->getRequest()->getParam('filename'))) {
69+
$this->messageManager->addErrorMessage(__('Please provide valid export file name'));
7370

74-
if ($directory->isFile($path)) {
75-
$this->file->deleteFile($path);
71+
return $resultRedirect;
72+
}
73+
$directoryWrite = $this->filesystem->getDirectoryWrite(DirectoryList::VAR_EXPORT);
74+
try {
75+
$directoryWrite->delete($directoryWrite->getAbsolutePath($fileName));
7676
$this->messageManager->addSuccessMessage(__('File %1 deleted', $fileName));
77-
} else {
78-
$this->messageManager->addErrorMessage(__('%1 is not a valid file', $fileName));
77+
} catch (ValidatorException $exception) {
78+
$this->messageManager->addErrorMessage(
79+
__('Sorry, but the data is invalid or the file is not uploaded.')
80+
);
81+
} catch (FileSystemException $exception) {
82+
$this->messageManager->addErrorMessage(
83+
__('Sorry, but the data is invalid or the file is not uploaded.')
84+
);
7985
}
8086
} catch (FileSystemException $exception) {
81-
$this->messageManager->addErrorMessage($exception->getMessage());
87+
$this->messageManager->addErrorMessage(__('There are no export file with such name %1', $fileName));
8288
}
8389

8490
return $resultRedirect;

0 commit comments

Comments
 (0)