Skip to content

Commit de9ec9d

Browse files
author
Bohdan Korablov
committed
Merge remote-tracking branch 'falcons/MAGETWO-67321' into MAGETWO-69983
2 parents 3d63d3e + 796cf10 commit de9ec9d

File tree

2 files changed

+117
-15
lines changed
  • app/code/Magento/CatalogImportExport

2 files changed

+117
-15
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
}

0 commit comments

Comments
 (0)