Skip to content

Commit 82416d7

Browse files
igrybkovigrybkov
authored
Merge pull request #1214 from magento-falcons/MAGETWO-69983
Fixed issues: - MAGETWO-69383 Site is down when scopes are not shared anymore through app/etc/config.php - MAGETWO-67159 [TD] Deployment commands improvements - MAGETWO-69535 Error during importing new list of scopes through shared file - MAGETWO-65658 [FT] User with restricted access can't log in to Admin in UpdateAdminUserEntityTest - MAGETWO-55429 Exception Logging Does Not Follow PSR-3 - MAGETWO-67321 Importing Products with images from external URL - MAGETWO-69983 Bugfixes delivery
2 parents ca01449 + 153c1d9 commit 82416d7

File tree

40 files changed

+1270
-292
lines changed

40 files changed

+1270
-292
lines changed

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

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,36 @@
66
namespace Magento\CatalogImportExport\Model\Import\Product\Validator;
77

88
use Magento\CatalogImportExport\Model\Import\Product\RowValidatorInterface;
9+
use Magento\Framework\App\ObjectManager;
10+
use Magento\Framework\Url\Validator;
911

1012
class Media extends AbstractImportValidator implements RowValidatorInterface
1113
{
14+
/**
15+
* @deprecated As this regexp doesn't give guarantee of correct url validation
16+
* @see \Magento\Framework\Url\Validator::isValid()
17+
*/
1218
const URL_REGEXP = '|^http(s)?://[a-z0-9-]+(.[a-z0-9-]+)*(:[0-9]+)?(/.*)?$|i';
1319

1420
const PATH_REGEXP = '#^(?!.*[\\/]\.{2}[\\/])(?!\.{2}[\\/])[-\w.\\/]+$#';
1521

1622
const ADDITIONAL_IMAGES = 'additional_images';
1723

24+
/**
25+
* The url validator. Checks if given url is valid.
26+
*
27+
* @var Validator
28+
*/
29+
private $validator;
30+
31+
/**
32+
* @param Validator $validator The url validator
33+
*/
34+
public function __construct(Validator $validator = null)
35+
{
36+
$this->validator = $validator ?: ObjectManager::getInstance()->get(Validator::class);
37+
}
38+
1839
/**
1940
* @deprecated
2041
* @see \Magento\CatalogImportExport\Model\Import\Product::getMultipleValueSeparator()
@@ -27,6 +48,8 @@ class Media extends AbstractImportValidator implements RowValidatorInterface
2748
/**
2849
* @param string $string
2950
* @return bool
51+
* @deprecated As this method doesn't give guarantee of correct url validation.
52+
* @see \Magento\Framework\Url\Validator::isValid() It provides better url validation.
3053
*/
3154
protected function checkValidUrl($string)
3255
{
@@ -64,7 +87,7 @@ public function isValid($value)
6487
$valid = true;
6588
foreach ($this->mediaAttributes as $attribute) {
6689
if (isset($value[$attribute]) && strlen($value[$attribute])) {
67-
if (!$this->checkPath($value[$attribute]) && !$this->checkValidUrl($value[$attribute])) {
90+
if (!$this->checkPath($value[$attribute]) && !$this->validator->isValid($value[$attribute])) {
6891
$this->_addMessages(
6992
[
7093
sprintf(
@@ -79,7 +102,7 @@ public function isValid($value)
79102
}
80103
if (isset($value[self::ADDITIONAL_IMAGES]) && strlen($value[self::ADDITIONAL_IMAGES])) {
81104
foreach (explode($this->context->getMultipleValueSeparator(), $value[self::ADDITIONAL_IMAGES]) as $image) {
82-
if (!$this->checkPath($image) && !$this->checkValidUrl($image)) {
105+
if (!$this->checkPath($image) && !$this->validator->isValid($image)) {
83106
$this->_addMessages(
84107
[
85108
sprintf(

‎app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Validator/MediaTest.php

Lines changed: 92 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
use Magento\CatalogImportExport\Model\Import\Product\Validator\Media;
1111
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
1212
use Magento\ImportExport\Model\Import;
13+
use Magento\Framework\Url\Validator;
14+
use PHPUnit_Framework_MockObject_MockObject as MockObject;
1315

1416
class MediaTest extends \PHPUnit_Framework_TestCase
1517
{
@@ -19,16 +21,35 @@ class MediaTest extends \PHPUnit_Framework_TestCase
1921
/** @var ObjectManagerHelper */
2022
protected $objectManagerHelper;
2123

24+
/**
25+
* @var Validator|MockObject
26+
*/
27+
private $validatorMock;
28+
2229
protected function setUp()
2330
{
24-
31+
$this->validatorMock = $this->getMockBuilder(Validator::class)
32+
->disableOriginalConstructor()
33+
->getMock();
34+
$contextMock = $this->getMockBuilder(Product::class)
35+
->disableOriginalConstructor()
36+
->getMock();
37+
$contextMock->expects($this->any())
38+
->method('getMultipleValueSeparator')
39+
->willReturn(Import::DEFAULT_GLOBAL_MULTI_VALUE_SEPARATOR);
40+
$contextMock->expects($this->any())
41+
->method('retrieveMessageTemplate')
42+
->with(Media::ERROR_INVALID_MEDIA_URL_OR_PATH)
43+
->willReturn('%s');
44+
2545
$this->objectManagerHelper = new ObjectManagerHelper($this);
2646
$this->media = $this->objectManagerHelper->getObject(
2747
Media::class,
2848
[
29-
49+
'validator' => $this->validatorMock
3050
]
3151
);
52+
$this->media->init($contextMock);
3253
}
3354

3455
public function testInit()
@@ -44,17 +65,8 @@ public function testInit()
4465
*/
4566
public function testIsValid($data, $expected)
4667
{
47-
$contextMock = $this->getMockBuilder(Product::class)
48-
->disableOriginalConstructor()
49-
->getMock();
50-
$contextMock->expects($this->any())
51-
->method('getMultipleValueSeparator')
52-
->willReturn(Import::DEFAULT_GLOBAL_MULTI_VALUE_SEPARATOR);
53-
$contextMock->expects($this->any())
54-
->method('retrieveMessageTemplate')
55-
->with(Media::ERROR_INVALID_MEDIA_URL_OR_PATH)
56-
->willReturn('%s');
57-
$this->media->init($contextMock);
68+
$this->validatorMock->expects($this->never())
69+
->method('isValid');
5870

5971
$result = $this->media->isValid($data);
6072
$this->assertEquals($expected['result'], $result);
@@ -76,6 +88,47 @@ public function testIsValidClearMessagesCall()
7688
$media->isValid([]);
7789
}
7890

91+
/**
92+
* @param array $data
93+
* @param array $expected
94+
* @dataProvider isValidAdditionalImagesPathDataProvider
95+
*/
96+
public function testIsValidAdditionalImagesPath($data, $expected)
97+
{
98+
if ($expected['result']) {
99+
$this->validatorMock->expects($this->never())
100+
->method('isValid');
101+
} else {
102+
$this->validatorMock->expects($this->once())
103+
->method('isValid')
104+
->with($data['additional_images'])
105+
->willReturn(false);
106+
}
107+
108+
$result = $this->media->isValid($data);
109+
$this->assertEquals($expected['result'], $result);
110+
$messages = $this->media->getMessages();
111+
$this->assertEquals($expected['messages'], $messages);
112+
}
113+
114+
/**
115+
* @param array $data
116+
* @param array $expected
117+
* @dataProvider isValidAdditionalImagesUrlDataProvider
118+
*/
119+
public function testIsValidAdditionalImagesUrl($data, $expected)
120+
{
121+
$this->validatorMock->expects($this->once())
122+
->method('isValid')
123+
->with($data['additional_images'])
124+
->willReturn($expected['result']);
125+
126+
$result = $this->media->isValid($data);
127+
$this->assertEquals($expected['result'], $result);
128+
$messages = $this->media->getMessages();
129+
$this->assertEquals($expected['messages'], $messages);
130+
}
131+
79132
/**
80133
* @return array
81134
*/
@@ -94,13 +147,39 @@ public function isMediaValidDataProvider()
94147
['_media_image' => 1],
95148
['result' => true,'messages' => []],
96149
],
150+
];
151+
}
152+
153+
/**
154+
* @return array
155+
*/
156+
public function isValidAdditionalImagesPathDataProvider()
157+
{
158+
return [
97159
'additional_images' => [
98160
['additional_images' => 'image1.png,image2.jpg'],
99161
['result' => true, 'messages' => []]
100162
],
101163
'additional_images_fail' => [
102164
['additional_images' => 'image1.png|image2.jpg|image3.gif'],
103165
['result' => false, 'messages' => [0 => 'additional_images']]
166+
],
167+
];
168+
}
169+
170+
/**
171+
* @return array
172+
*/
173+
public function isValidAdditionalImagesUrlDataProvider()
174+
{
175+
return [
176+
'additional_images_wrong_domain' => [
177+
['additional_images' => 'https://example/images/some-name.jpg'],
178+
['result' => false, 'messages' => [0 => 'additional_images']],
179+
],
180+
'additional_images_url_multiple_underscores' => [
181+
['additional_images' => 'https://example.com/images/some-name__with___multiple____underscores.jpg'],
182+
['result' => true, 'messages' => []]
104183
]
105184
];
106185
}

‎app/code/Magento/Config/Model/Config/Importer.php

Lines changed: 11 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@
1313
use Magento\Framework\App\State;
1414
use Magento\Framework\Config\ScopeInterface;
1515
use Magento\Framework\Exception\State\InvalidTransitionException;
16-
use Magento\Framework\Flag\FlagResource;
17-
use Magento\Framework\FlagFactory;
16+
use Magento\Framework\FlagManager;
1817
use Magento\Framework\Stdlib\ArrayUtils;
1918

2019
/**
@@ -32,18 +31,11 @@ class Importer implements ImporterInterface
3231
const FLAG_CODE = 'system_config_snapshot';
3332

3433
/**
35-
* The flag factory.
34+
* The flag manager.
3635
*
37-
* @var FlagFactory
36+
* @var FlagManager
3837
*/
39-
private $flagFactory;
40-
41-
/**
42-
* The flag resource.
43-
*
44-
* @var FlagResource
45-
*/
46-
private $flagResource;
38+
private $flagManager;
4739

4840
/**
4941
* An array utils.
@@ -81,25 +73,22 @@ class Importer implements ImporterInterface
8173
private $saveProcessor;
8274

8375
/**
84-
* @param FlagFactory $flagFactory The flag factory
85-
* @param FlagResource $flagResource The flag resource
76+
* @param FlagManager $flagManager The flag manager
8677
* @param ArrayUtils $arrayUtils An array utils
8778
* @param SaveProcessor $saveProcessor Saves configuration data
8879
* @param ScopeConfigInterface $scopeConfig The application config storage.
8980
* @param State $state The application scope to run
9081
* @param ScopeInterface $scope The application scope
9182
*/
9283
public function __construct(
93-
FlagFactory $flagFactory,
94-
FlagResource $flagResource,
84+
FlagManager $flagManager,
9585
ArrayUtils $arrayUtils,
9686
SaveProcessor $saveProcessor,
9787
ScopeConfigInterface $scopeConfig,
9888
State $state,
9989
ScopeInterface $scope
10090
) {
101-
$this->flagFactory = $flagFactory;
102-
$this->flagResource = $flagResource;
91+
$this->flagManager = $flagManager;
10392
$this->arrayUtils = $arrayUtils;
10493
$this->saveProcessor = $saveProcessor;
10594
$this->scopeConfig = $scopeConfig;
@@ -118,13 +107,10 @@ public function import(array $data)
118107
$currentScope = $this->scope->getCurrentScope();
119108

120109
try {
121-
$flag = $this->flagFactory->create(['data' => ['flag_code' => static::FLAG_CODE]]);
122-
$this->flagResource->load($flag, static::FLAG_CODE, 'flag_code');
123-
$previouslyProcessedData = $flag->getFlagData() ?: [];
124-
110+
$savedFlag = $this->flagManager->getFlagData(static::FLAG_CODE) ?: [];
125111
$changedData = array_replace_recursive(
126-
$this->arrayUtils->recursiveDiff($previouslyProcessedData, $data),
127-
$this->arrayUtils->recursiveDiff($data, $previouslyProcessedData)
112+
$this->arrayUtils->recursiveDiff($savedFlag, $data),
113+
$this->arrayUtils->recursiveDiff($data, $savedFlag)
128114
);
129115

130116
/**
@@ -142,8 +128,7 @@ public function import(array $data)
142128
$this->saveProcessor->process($changedData);
143129
});
144130

145-
$flag->setFlagData($data);
146-
$this->flagResource->save($flag);
131+
$this->flagManager->saveFlag(static::FLAG_CODE, $data);
147132
} catch (\Exception $e) {
148133
throw new InvalidTransitionException(__('%1', $e->getMessage()), $e);
149134
} finally {

‎app/code/Magento/Config/Model/PreparedValueFactory.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ public function __construct(
8080
* @return ValueInterface
8181
* @throws RuntimeException If Value can not be created
8282
* @see ValueInterface
83-
* @see Value
8483
*/
8584
public function create($path, $value, $scope, $scopeCode = null)
8685
{

0 commit comments

Comments
 (0)