MAGETWO-64465: Investigate and fix post-refactoring issues

rganin · rganin · commit 14a722278fe3 · 2017-03-28T09:55:33.000+03:00
diff --git a/app/code/Magento/Catalog/Model/Product/Option/Type/File.php b/app/code/Magento/Catalog/Model/Product/Option/Type/File.php
@@ -6,6 +6,7 @@
 namespace Magento\Catalog\Model\Product\Option\Type;
 
 use Magento\Framework\App\Filesystem\DirectoryList;
+use Magento\Framework\Exception\SerializationException;
 use Magento\Framework\Filesystem;
 use Magento\Framework\Exception\LocalizedException;
 use Magento\Catalog\Model\Product\Exception as ProductException;
@@ -316,7 +317,7 @@ public function getFormattedOptionValue($optionValue)
     {
         if ($this->_formattedOptionValue === null) {
             try {
-                $value = $this->serializer->unserialize($optionValue);
+                $value = $this->unserializeJsonValue($optionValue);
 
                 $customOptionUrlParams = $this->getCustomOptionUrlParams() ? $this->getCustomOptionUrlParams() : [
                     'id' => $this->getConfigurationItemOption()->getId(),
@@ -402,7 +403,7 @@ public function getPrintableOptionValue($optionValue)
     public function getEditableOptionValue($optionValue)
     {
         try {
-            $value = $this->serializer->unserialize($optionValue);
+            $value = $this->unserializeJsonValue($optionValue);
             return sprintf(
                 '%s [%d]',
                 $this->_escaper->escapeHtml($value['title']),
@@ -431,7 +432,7 @@ public function parseOptionValue($optionValue, $productOptionValues)
             $confItemOptionId = $matches[1];
             $option = $this->_itemOptionFactory->create()->load($confItemOptionId);
             try {
-                $this->serializer->unserialize($option->getValue());
+                $this->unserializeJsonValue($option->getValue());
                 return $option->getValue();
             } catch (\Exception $e) {
                 return null;
@@ -441,6 +442,23 @@ public function parseOptionValue($optionValue, $productOptionValues)
         }
     }
 
+    /**
+     * Validate Json serialized value
+     *
+     * @param string $jsonValue - JSON serialized value
+     * @return mixed
+     * @throws SerializationException
+     */
+    private function unserializeJsonValue($jsonValue)
+    {
+        $value = $this->serializer->unserialize($jsonValue);
+        if (json_last_error() === JSON_ERROR_NONE) {
+            return $value;
+        } else {
+            throw new SerializationException(__('Invalid JSON value.'));
+        }
+    }
+
     /**
      * Prepare option value for info buy request
      *
@@ -450,8 +468,7 @@ public function parseOptionValue($optionValue, $productOptionValues)
     public function prepareOptionValueForRequest($optionValue)
     {
         try {
-            $result = $this->serializer->unserialize($optionValue);
-            return $result;
+            return $this->unserializeJsonValue($optionValue);
         } catch (\Exception $e) {
             return null;
         }
diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Type/FileTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Product/Option/Type/FileTest.php
@@ -7,6 +7,7 @@
 
 use Magento\Catalog\Model\Product\Configuration\Item\Option\OptionInterface;
 use Magento\Framework\App\Filesystem\DirectoryList;
+use Magento\Framework\Exception\SerializationException;
 use Magento\Framework\Filesystem;
 use Magento\Framework\Filesystem\Directory\ReadInterface;
 use Magento\Framework\Filesystem\DriverPool;
@@ -53,6 +54,11 @@ class FileTest extends \PHPUnit_Framework_TestCase
      */
     private $escaper;
 
+    /**
+     * @var \Magento\Quote\Model\Quote\Item\OptionFactory|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $itemOptionFactoryMock;
+
     protected function setUp()
     {
         $this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
@@ -81,6 +87,11 @@ protected function setUp()
             ->disableOriginalConstructor()
             ->getMock();
 
+        $this->itemOptionFactoryMock = $this->getMockBuilder(\Magento\Quote\Model\Quote\Item\OptionFactory::class)
+            ->setMethods(['create'])
+            ->disableOriginalConstructor()
+            ->getMock();
+
         $this->coreFileStorageDatabase = $this->getMock(
             \Magento\MediaStorage\Helper\File\Storage\Database::class,
             ['copyFile'],
@@ -102,7 +113,8 @@ protected function getFileObject()
                 'coreFileStorageDatabase' => $this->coreFileStorageDatabase,
                 'serializer' => $this->serializer,
                 'urlBuilder' => $this->urlBuilder,
-                'escaper' => $this->escaper
+                'escaper' => $this->escaper,
+                'itemOptionFactory' => $this->itemOptionFactoryMock,
             ]
         );
     }
@@ -222,6 +234,141 @@ public function testGetFormattedOptionValue()
         $fileObject->getFormattedOptionValue($optionValue);
     }
 
+    public function testGetFormattedOptionValueInvalid()
+    {
+        $optionValue = 'invalid json option value...';
+        $this->serializer->expects($this->once())
+            ->method('unserialize')
+            ->willThrowException(new SerializationException(__('Invalid JSON value.')));
+        $this->assertEquals($optionValue, $this->getFileObject()->getFormattedOptionValue($optionValue));
+    }
+
+    public function testGetEditableOptionValue()
+    {
+        $configurationItemOption = $this->getMockBuilder(\Magento\Catalog\Model\Product\Configuration\Item\Option\OptionInterface::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['getId', 'getValue'])
+            ->getMock();
+        $configurationItemOption->expects($this->once())
+            ->method('getId')
+            ->will($this->returnValue(2));
+        $fileObject = $this->getFileObject()->setData('configuration_item_option', $configurationItemOption);
+        $optionTitle = 'Option Title';
+        $optionValue = json_encode(['title' => $optionTitle]);
+        $this->serializer->expects($this->once())
+            ->method('unserialize')
+            ->with($optionValue)
+            ->willReturn(json_decode($optionValue, true));
+        $this->escaper->expects($this->once())
+            ->method('escapeHtml')
+            ->with($optionTitle)
+            ->will($this->returnValue($optionTitle));
+
+        $this->assertEquals('Option Title [2]', $fileObject->getEditableOptionValue($optionValue));
+    }
+
+    public function testGetEditableOptionValueInvalid()
+    {
+        $fileObject = $this->getFileObject();
+        $optionTitle = 'Option Title';
+        $optionValue = json_encode(['title' => $optionTitle]);
+        $this->serializer->expects($this->once())
+            ->method('unserialize')
+            ->with($optionValue)
+            ->willThrowException(new SerializationException(__('Invalid JSON value.')));
+        $this->escaper->expects($this->never())
+            ->method('escapeHtml');
+
+        $this->assertEquals($optionValue, $fileObject->getEditableOptionValue($optionValue));
+    }
+
+    public function testParseOptionValue()
+    {
+        $optionTitle = 'Option Title';
+        $optionValue = json_encode(['title' => $optionTitle]);
+
+        $userInput = 'Option [2]';
+        $fileObject = $this->getFileObject();
+
+        $itemMock = $this->getMockBuilder(\Magento\Quote\Model\Quote\Item\Option::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['load', 'getValue'])
+            ->getMock();
+
+        $itemMock->expects($this->any())
+            ->method('load')
+            ->will($this->returnSelf());
+
+        $itemMock->expects($this->any())
+            ->method('getValue')
+            ->will($this->returnValue($optionValue));
+
+        $this->itemOptionFactoryMock->expects($this->any())
+            ->method('create')
+            ->will($this->returnValue($itemMock));
+
+        $this->assertEquals($optionValue, $fileObject->parseOptionValue($userInput, []));
+    }
+
+    public function testParseOptionValueNoId()
+    {
+        $optionValue = 'value';
+
+        $userInput = 'Option [xx]';
+        $fileObject = $this->getFileObject();
+
+        $itemMock = $this->getMockBuilder(\Magento\Quote\Model\Quote\Item\Option::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['load', 'getValue'])
+            ->getMock();
+
+        $itemMock->expects($this->any())
+            ->method('load')
+            ->will($this->returnSelf());
+
+        $itemMock->expects($this->any())
+            ->method('getValue')
+            ->will($this->returnValue($optionValue));
+
+        $this->itemOptionFactoryMock->expects($this->any())
+            ->method('create')
+            ->will($this->returnValue($itemMock));
+
+        $this->assertEquals(null, $fileObject->parseOptionValue($userInput, []));
+    }
+
+    public function testParseOptionValueInvalid()
+    {
+        $optionValue = 'Invalid json serialized value...';
+
+        $userInput = 'Option [2]';
+        $fileObject = $this->getFileObject();
+
+        $itemMock = $this->getMockBuilder(\Magento\Quote\Model\Quote\Item\Option::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['load', 'getValue'])
+            ->getMock();
+
+        $itemMock->expects($this->any())
+            ->method('load')
+            ->will($this->returnSelf());
+
+        $itemMock->expects($this->any())
+            ->method('getValue')
+            ->will($this->returnValue($optionValue));
+
+        $this->itemOptionFactoryMock->expects($this->any())
+            ->method('create')
+            ->will($this->returnValue($itemMock));
+
+        $this->serializer->expects($this->once())
+            ->method('unserialize')
+            ->with($optionValue)
+            ->willThrowException(new SerializationException(__('Invalid JSON value.')));
+
+        $this->assertEquals(null, $fileObject->parseOptionValue($userInput, []));
+    }
+
     public function testPrepareOptionValueForRequest()
     {
         $optionValue = 'string';
diff --git a/app/code/Magento/Quote/Model/Quote/Item/Updater.php b/app/code/Magento/Quote/Model/Quote/Item/Updater.php
@@ -105,7 +105,7 @@ public function update(Item $item, array $info)
      *
      * @param array $info
      * @param Item $item
-     * @return array
+     * @return void
      */
     protected function setCustomPrice(array $info, Item $item)
     {

Original file line number	Diff line number	Diff line change
`@@ -105,7 +105,7 @@ public function update(Item $item, array $info)`
`105`	`105`	`*`
`106`	`106`	`* @param array $info`
`107`	`107`	`* @param Item $item`
`108`		`- * @return array`
	`108`	`+ * @return void`
`109`	`109`	`*/`
`110`	`110`	`protected function setCustomPrice(array $info, Item $item)`
`111`	`111`	`{`