Merge pull request #1018 from magento-troll/MAGETWO-64465

rganin · web-flow · commit 8b656e2f5f86 · 2017-04-12T11:05:07.000+03:00
MAGETWO-64465: Investigate and fix post-refactoring issues
diff --git a/app/code/Magento/Catalog/Model/Product/Option/Type/File.php b/app/code/Magento/Catalog/Model/Product/Option/Type/File.php
@@ -315,22 +315,21 @@ public function prepareForCart()
     public function getFormattedOptionValue($optionValue)
     {
         if ($this->_formattedOptionValue === null) {
-            try {
-                $value = $this->serializer->unserialize($optionValue);
-
-                $customOptionUrlParams = $this->getCustomOptionUrlParams() ? $this->getCustomOptionUrlParams() : [
+            $value = $this->serializer->unserialize($optionValue);
+            if ($value === null) {
+                return $optionValue;
+            }
+            $customOptionUrlParams = $this->getCustomOptionUrlParams()
+                ? $this->getCustomOptionUrlParams()
+                : [
                     'id' => $this->getConfigurationItemOption()->getId(),
                     'key' => $value['secret_key']
                 ];
 
-                $value['url'] = ['route' => $this->_customOptionDownloadUrl, 'params' => $customOptionUrlParams];
+            $value['url'] = ['route' => $this->_customOptionDownloadUrl, 'params' => $customOptionUrlParams];
 
-                $this->_formattedOptionValue = $this->_getOptionHtml($value);
-                $this->getConfigurationItemOption()->setValue($this->serializer->serialize($value));
-                return $this->_formattedOptionValue;
-            } catch (\Exception $e) {
-                return $optionValue;
-            }
+            $this->_formattedOptionValue = $this->_getOptionHtml($value);
+            $this->getConfigurationItemOption()->setValue($this->serializer->serialize($value));
         }
         return $this->_formattedOptionValue;
     }
@@ -401,16 +400,15 @@ public function getPrintableOptionValue($optionValue)
      */
     public function getEditableOptionValue($optionValue)
     {
-        try {
-            $value = $this->serializer->unserialize($optionValue);
+        $unserializedValue = $this->serializer->unserialize($optionValue);
+        if ($unserializedValue !== null) {
             return sprintf(
                 '%s [%d]',
-                $this->_escaper->escapeHtml($value['title']),
+                $this->_escaper->escapeHtml($unserializedValue['title']),
                 $this->getConfigurationItemOption()->getId()
             );
-        } catch (\Exception $e) {
-            return $optionValue;
         }
+        return $optionValue;
     }
 
     /**
@@ -430,15 +428,11 @@ public function parseOptionValue($optionValue, $productOptionValues)
         if (preg_match('/\[([0-9]+)\]/', $optionValue, $matches)) {
             $confItemOptionId = $matches[1];
             $option = $this->_itemOptionFactory->create()->load($confItemOptionId);
-            try {
-                $this->serializer->unserialize($option->getValue());
+            if ($this->serializer->unserialize($option->getValue()) !== null) {
                 return $option->getValue();
-            } catch (\Exception $e) {
-                return null;
             }
-        } else {
-            return null;
         }
+        return null;
     }
 
     /**
@@ -449,12 +443,11 @@ public function parseOptionValue($optionValue, $productOptionValues)
      */
     public function prepareOptionValueForRequest($optionValue)
     {
-        try {
-            $result = $this->serializer->unserialize($optionValue);
-            return $result;
-        } catch (\Exception $e) {
-            return null;
+        $unserializedValue = $this->serializer->unserialize($optionValue);
+        if ($unserializedValue !== null) {
+            return $unserializedValue;
         }
+        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);
@@ -71,6 +77,7 @@ protected function setUp()
 
         $this->serializer = $this->getMockBuilder(\Magento\Framework\Serialize\Serializer\Json::class)
             ->disableOriginalConstructor()
+            ->setMethods(['serialize', 'unserialize'])
             ->getMock();
 
         $this->urlBuilder = $this->getMockBuilder(\Magento\Catalog\Model\Product\Option\UrlBuilder::class)
@@ -81,13 +88,38 @@ 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'],
             [],
             '',
             false
         );
+
+        $this->serializer->expects($this->any())
+            ->method('unserialize')
+            ->will(
+                $this->returnCallback(
+                    function ($value) {
+                        return json_decode($value, true);
+                    }
+                )
+            );
+
+        $this->serializer->expects($this->any())
+            ->method('serialize')
+            ->will(
+                $this->returnCallback(
+                    function ($value) {
+                        return json_encode($value);
+                    }
+                )
+            );
     }
 
     /**
@@ -102,7 +134,8 @@ protected function getFileObject()
                 'coreFileStorageDatabase' => $this->coreFileStorageDatabase,
                 'serializer' => $this->serializer,
                 'urlBuilder' => $this->urlBuilder,
-                'escaper' => $this->escaper
+                'escaper' => $this->escaper,
+                'itemOptionFactory' => $this->itemOptionFactoryMock,
             ]
         );
     }
@@ -222,17 +255,134 @@ public function testGetFormattedOptionValue()
         $fileObject->getFormattedOptionValue($optionValue);
     }
 
-    public function testPrepareOptionValueForRequest()
+    public function testGetFormattedOptionValueInvalid()
     {
-        $optionValue = 'string';
-        $resultValue = ['result'];
-        $fileObject = $this->getFileObject();
+        $optionValue = 'invalid json option 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($resultValue);
+            ->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();
+        $optionValue = '#invalid jSoN*(&@#^$(*&';
+        $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->assertEquals(null, $fileObject->parseOptionValue($userInput, []));
+    }
+
+    public function testPrepareOptionValueForRequest()
+    {
+        $resultValue = ['result'];
+        $optionValue = json_encode($resultValue);
+        $fileObject = $this->getFileObject();
         $this->assertEquals($resultValue, $fileObject->prepareOptionValueForRequest($optionValue));
     }
 }
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`	`{`