Merge remote-tracking branch 'mainline/develop' into MAGETWO-69261

magento · Jun 23, 2017 · 87bd339 · 87bd339
2 parents aa5d948 + d816ce7
commit 87bd339
Show file tree

Hide file tree

Showing 58 changed files with 1,841 additions and 491 deletions.
diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product/Validator/Media.php b/app/code/Magento/CatalogImportExport/Model/Import/Product/Validator/Media.php
@@ -6,15 +6,36 @@
 namespace Magento\CatalogImportExport\Model\Import\Product\Validator;
 
 use Magento\CatalogImportExport\Model\Import\Product\RowValidatorInterface;
+use Magento\Framework\App\ObjectManager;
+use Magento\Framework\Url\Validator;
 
 class Media extends AbstractImportValidator implements RowValidatorInterface
 {
+    /**
+     * @deprecated As this regexp doesn't give guarantee of correct url validation
+     * @see \Magento\Framework\Url\Validator::isValid()
+     */
     const URL_REGEXP = '|^http(s)?://[a-z0-9-]+(.[a-z0-9-]+)*(:[0-9]+)?(/.*)?$|i';
 
     const PATH_REGEXP = '#^(?!.*[\\/]\.{2}[\\/])(?!\.{2}[\\/])[-\w.\\/]+$#';
 
     const ADDITIONAL_IMAGES = 'additional_images';
 
+    /**
+     * The url validator. Checks if given url is valid.
+     *
+     * @var Validator
+     */
+    private $validator;
+
+    /**
+     * @param Validator $validator The url validator
+     */
+    public function __construct(Validator $validator = null)
+    {
+        $this->validator = $validator ?: ObjectManager::getInstance()->get(Validator::class);
+    }
+
     /**
      * @deprecated
      * @see \Magento\CatalogImportExport\Model\Import\Product::getMultipleValueSeparator()
@@ -27,6 +48,8 @@ class Media extends AbstractImportValidator implements RowValidatorInterface
     /**
      * @param string $string
      * @return bool
+     * @deprecated As this method doesn't give guarantee of correct url validation.
+     * @see \Magento\Framework\Url\Validator::isValid() It provides better url validation.
      */
     protected function checkValidUrl($string)
     {
@@ -64,7 +87,7 @@ public function isValid($value)
         $valid = true;
         foreach ($this->mediaAttributes as $attribute) {
             if (isset($value[$attribute]) && strlen($value[$attribute])) {
-                if (!$this->checkPath($value[$attribute]) && !$this->checkValidUrl($value[$attribute])) {
+                if (!$this->checkPath($value[$attribute]) && !$this->validator->isValid($value[$attribute])) {
                     $this->_addMessages(
                         [
                             sprintf(
@@ -79,7 +102,7 @@ public function isValid($value)
         }
         if (isset($value[self::ADDITIONAL_IMAGES]) && strlen($value[self::ADDITIONAL_IMAGES])) {
             foreach (explode($this->context->getMultipleValueSeparator(), $value[self::ADDITIONAL_IMAGES]) as $image) {
-                if (!$this->checkPath($image) && !$this->checkValidUrl($image)) {
+                if (!$this->checkPath($image) && !$this->validator->isValid($image)) {
                     $this->_addMessages(
                         [
                             sprintf(

diff --git a/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Validator/MediaTest.php b/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Validator/MediaTest.php
@@ -10,6 +10,8 @@
 use Magento\CatalogImportExport\Model\Import\Product\Validator\Media;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
 use Magento\ImportExport\Model\Import;
+use Magento\Framework\Url\Validator;
+use PHPUnit_Framework_MockObject_MockObject as MockObject;
 
 class MediaTest extends \PHPUnit_Framework_TestCase
 {
@@ -19,16 +21,35 @@ class MediaTest extends \PHPUnit_Framework_TestCase
     /** @var ObjectManagerHelper */
     protected $objectManagerHelper;
 
+    /**
+     * @var Validator|MockObject
+     */
+    private $validatorMock;
+
     protected function setUp()
     {
-
+        $this->validatorMock = $this->getMockBuilder(Validator::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $contextMock = $this->getMockBuilder(Product::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $contextMock->expects($this->any())
+            ->method('getMultipleValueSeparator')
+            ->willReturn(Import::DEFAULT_GLOBAL_MULTI_VALUE_SEPARATOR);
+        $contextMock->expects($this->any())
+            ->method('retrieveMessageTemplate')
+            ->with(Media::ERROR_INVALID_MEDIA_URL_OR_PATH)
+            ->willReturn('%s');
+
         $this->objectManagerHelper = new ObjectManagerHelper($this);
         $this->media = $this->objectManagerHelper->getObject(
             Media::class,
             [
-
+                'validator' => $this->validatorMock
             ]
         );
+        $this->media->init($contextMock);
     }
 
     public function testInit()
@@ -44,17 +65,8 @@ public function testInit()
      */
     public function testIsValid($data, $expected)
     {
-        $contextMock = $this->getMockBuilder(Product::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-        $contextMock->expects($this->any())
-            ->method('getMultipleValueSeparator')
-            ->willReturn(Import::DEFAULT_GLOBAL_MULTI_VALUE_SEPARATOR);
-        $contextMock->expects($this->any())
-            ->method('retrieveMessageTemplate')
-            ->with(Media::ERROR_INVALID_MEDIA_URL_OR_PATH)
-            ->willReturn('%s');
-        $this->media->init($contextMock);
+        $this->validatorMock->expects($this->never())
+            ->method('isValid');
 
         $result = $this->media->isValid($data);
         $this->assertEquals($expected['result'], $result);
@@ -76,6 +88,47 @@ public function testIsValidClearMessagesCall()
         $media->isValid([]);
     }
 
+    /**
+     * @param array $data
+     * @param array $expected
+     * @dataProvider isValidAdditionalImagesPathDataProvider
+     */
+    public function testIsValidAdditionalImagesPath($data, $expected)
+    {
+        if ($expected['result']) {
+            $this->validatorMock->expects($this->never())
+                ->method('isValid');
+        } else {
+            $this->validatorMock->expects($this->once())
+                ->method('isValid')
+                ->with($data['additional_images'])
+                ->willReturn(false);
+        }
+
+        $result = $this->media->isValid($data);
+        $this->assertEquals($expected['result'], $result);
+        $messages = $this->media->getMessages();
+        $this->assertEquals($expected['messages'], $messages);
+    }
+
+    /**
+     * @param array $data
+     * @param array $expected
+     * @dataProvider isValidAdditionalImagesUrlDataProvider
+     */
+    public function testIsValidAdditionalImagesUrl($data, $expected)
+    {
+        $this->validatorMock->expects($this->once())
+            ->method('isValid')
+            ->with($data['additional_images'])
+            ->willReturn($expected['result']);
+
+        $result = $this->media->isValid($data);
+        $this->assertEquals($expected['result'], $result);
+        $messages = $this->media->getMessages();
+        $this->assertEquals($expected['messages'], $messages);
+    }
+
     /**
      * @return array
      */
@@ -94,13 +147,39 @@ public function isMediaValidDataProvider()
                 ['_media_image' => 1],
                 ['result' => true,'messages' => []],
             ],
+        ];
+    }
+
+    /**
+     * @return array
+     */
+    public function isValidAdditionalImagesPathDataProvider()
+    {
+        return [
             'additional_images' => [
                 ['additional_images' => 'image1.png,image2.jpg'],
                 ['result' => true, 'messages' => []]
             ],
             'additional_images_fail' => [
                 ['additional_images' => 'image1.png|image2.jpg|image3.gif'],
                 ['result' => false, 'messages' => [0 => 'additional_images']]
+            ],
+        ];
+    }
+
+    /**
+     * @return array
+     */
+    public function isValidAdditionalImagesUrlDataProvider()
+    {
+        return [
+            'additional_images_wrong_domain' => [
+                ['additional_images' => 'https://example/images/some-name.jpg'],
+                ['result' => false, 'messages' => [0 => 'additional_images']],
+            ],
+            'additional_images_url_multiple_underscores' => [
+                ['additional_images' => 'https://example.com/images/some-name__with___multiple____underscores.jpg'],
+                ['result' => true, 'messages' => []]
             ]
         ];
     }