From a6a7f20f6c3d9e1ab1c044d220188cfd731f6ae3 Mon Sep 17 00:00:00 2001 From: Olga Kopylova <okopylova@magento.com> Date: Wed, 19 Oct 2016 15:14:14 -0500 Subject: [PATCH] MAGETWO-58691: Refactor Module_Quote, Module_Sales - replaced native functions with SerializerInterface --- .../Magento/Quote/Model/QueryResolver.php | 15 +++- .../Model/Quote/Address/Total/Collector.php | 7 +- .../Test/Unit/Model/QueryResolverTest.php | 75 ++++++++++++------- .../Magento/Sales/Model/Config/Ordered.php | 15 +++- .../Sales/Model/Order/Total/Config/Base.php | 7 +- .../Model/Order/Total/Config/BaseTest.php | 32 ++++++-- 6 files changed, 105 insertions(+), 46 deletions(-) diff --git a/app/code/Magento/Quote/Model/QueryResolver.php b/app/code/Magento/Quote/Model/QueryResolver.php index cfc1480feb6..216e644b767 100644 --- a/app/code/Magento/Quote/Model/QueryResolver.php +++ b/app/code/Magento/Quote/Model/QueryResolver.php @@ -5,8 +5,10 @@ */ namespace Magento\Quote\Model; +use Magento\Framework\App\ObjectManager; use Magento\Framework\Config\CacheInterface; use Magento\Framework\App\ResourceConnection\ConfigInterface; +use Magento\Framework\Serialize\SerializerInterface; class QueryResolver { @@ -36,20 +38,27 @@ class QueryResolver * @var array */ private $cacheTags = []; + /** + * @var SerializerInterface + */ + private $serializer; /** * @param ConfigInterface $config * @param CacheInterface $cache * @param string $cacheId + * @param SerializerInterface $serializer */ public function __construct( ConfigInterface $config, CacheInterface $cache, - $cacheId = 'connection_config_cache' + $cacheId = 'connection_config_cache', + SerializerInterface $serializer = null ) { $this->config = $config; $this->cache = $cache; $this->cacheId = $cacheId; + $this->serializer = $serializer ?: ObjectManager::getInstance()->get(SerializerInterface::class); } /** @@ -75,9 +84,9 @@ class QueryResolver if (false === $data) { $singleQuery = $this->config->getConnectionName('checkout_setup') == 'default' ? true : false; $data['checkout'] = $singleQuery; - $this->cache->save(serialize($data), $this->cacheId, $this->cacheTags); + $this->cache->save($this->serializer->serialize($data), $this->cacheId, $this->cacheTags); } else { - $data = unserialize($data); + $data = $this->serializer->unserialize($data); } $this->merge($data); } diff --git a/app/code/Magento/Quote/Model/Quote/Address/Total/Collector.php b/app/code/Magento/Quote/Model/Quote/Address/Total/Collector.php index 8e62d1b942f..0b3dbc318b2 100644 --- a/app/code/Magento/Quote/Model/Quote/Address/Total/Collector.php +++ b/app/code/Magento/Quote/Model/Quote/Address/Total/Collector.php @@ -4,6 +4,7 @@ * See COPYING.txt for license details. */ namespace Magento\Quote\Model\Quote\Address\Total; +use Magento\Framework\Serialize\SerializerInterface; /** * Address Total Collector model @@ -69,6 +70,7 @@ class Collector extends \Magento\Sales\Model\Config\Ordered * @param \Magento\Quote\Model\Quote\Address\TotalFactory $totalFactory * @param mixed $sourceData * @param mixed $store + * @param SerializerInterface $serializer */ public function __construct( \Magento\Framework\App\Cache\Type\Config $configCacheType, @@ -78,11 +80,12 @@ class Collector extends \Magento\Sales\Model\Config\Ordered \Magento\Store\Model\StoreManagerInterface $storeManager, \Magento\Quote\Model\Quote\Address\TotalFactory $totalFactory, $sourceData = null, - $store = null + $store = null, + SerializerInterface $serializer = null ) { $this->_scopeConfig = $scopeConfig; $this->_totalFactory = $totalFactory; - parent::__construct($configCacheType, $logger, $salesConfig, $sourceData); + parent::__construct($configCacheType, $logger, $salesConfig, $sourceData, $serializer); $this->_store = $store ?: $storeManager->getStore(); $this->_initModels()->_initCollectors()->_initRetrievers(); } diff --git a/app/code/Magento/Quote/Test/Unit/Model/QueryResolverTest.php b/app/code/Magento/Quote/Test/Unit/Model/QueryResolverTest.php index a2075b1161f..eb6828afff4 100644 --- a/app/code/Magento/Quote/Test/Unit/Model/QueryResolverTest.php +++ b/app/code/Magento/Quote/Test/Unit/Model/QueryResolverTest.php @@ -6,83 +6,100 @@ namespace Magento\Quote\Test\Unit\Model; +use Magento\Framework\Serialize\SerializerInterface; + class QueryResolverTest extends \PHPUnit_Framework_TestCase { /** * @var \Magento\Quote\Model\QueryResolver */ - protected $quoteResolver; + private $quoteResolver; /** * @var \PHPUnit_Framework_MockObject_MockObject */ - protected $configMock; + private $configMock; /** * @var \PHPUnit_Framework_MockObject_MockObject */ - protected $cacheMock; + private $cacheMock; + + /** + * @var SerializerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $serializer; protected function setUp() { $this->configMock = $this->getMock(\Magento\Framework\App\ResourceConnection\ConfigInterface::class); $this->cacheMock = $this->getMock(\Magento\Framework\Config\CacheInterface::class); + $this->serializer = $this->getMockForAbstractClass(SerializerInterface::class); $this->quoteResolver = new \Magento\Quote\Model\QueryResolver( $this->configMock, $this->cacheMock, - 'connection_config_cache' + 'connection_config_cache', + $this->serializer ); - } public function testIsSingleQueryWhenDataWereCached() { - $queryData['checkout'] = true; + $serializedData = '{"checkout":true}'; + $data = ['checkout' => true]; $this->cacheMock ->expects($this->once()) ->method('load') ->with('connection_config_cache') - ->willReturn(serialize($queryData)); + ->willReturn($serializedData); + $this->serializer->expects($this->once()) + ->method('unserialize') + ->with($serializedData) + ->willReturn($data); $this->assertTrue($this->quoteResolver->isSingleQuery()); } - public function testIsSingleQueryWhenDataNotCached() + /** + * @param string $connectionName + * @param bool $isSingleQuery + * + * @dataProvider isSingleQueryWhenDataNotCachedDataProvider + */ + public function testIsSingleQueryWhenDataNotCached($connectionName, $isSingleQuery) { - $queryData['checkout'] = true; + $data = ['checkout' => $isSingleQuery]; + $serializedData = '{"checkout":true}'; $this->cacheMock ->expects($this->once()) ->method('load') ->with('connection_config_cache') ->willReturn(false); + $this->serializer->expects($this->never()) + ->method('unserialize'); $this->configMock ->expects($this->once()) ->method('getConnectionName') ->with('checkout_setup') - ->willReturn('default'); + ->willReturn($connectionName); + $this->serializer->expects($this->once()) + ->method('serialize') + ->with($data) + ->willReturn($serializedData); $this->cacheMock ->expects($this->once()) ->method('save') - ->with(serialize($queryData), 'connection_config_cache', []); - $this->assertTrue($this->quoteResolver->isSingleQuery()); + ->with($serializedData, 'connection_config_cache', []); + $this->assertEquals($isSingleQuery, $this->quoteResolver->isSingleQuery()); } - public function testIsSingleQueryWhenSeveralConnectionsExist() + /** + * @return array + */ + public function isSingleQueryWhenDataNotCachedDataProvider() { - $queryData['checkout'] = false; - $this->cacheMock - ->expects($this->once()) - ->method('load') - ->with('connection_config_cache') - ->willReturn(false); - $this->configMock - ->expects($this->once()) - ->method('getConnectionName') - ->with('checkout_setup') - ->willReturn('checkout'); - $this->cacheMock - ->expects($this->once()) - ->method('save') - ->with(serialize($queryData), 'connection_config_cache', []); - $this->assertFalse($this->quoteResolver->isSingleQuery()); + return [ + ['default', true], + ['checkout', false], + ]; } } diff --git a/app/code/Magento/Sales/Model/Config/Ordered.php b/app/code/Magento/Sales/Model/Config/Ordered.php index 7ea7d1f8cc5..c59bd097bf5 100644 --- a/app/code/Magento/Sales/Model/Config/Ordered.php +++ b/app/code/Magento/Sales/Model/Config/Ordered.php @@ -4,6 +4,8 @@ * See COPYING.txt for license details. */ namespace Magento\Sales\Model\Config; +use Magento\Framework\App\ObjectManager; +use Magento\Framework\Serialize\SerializerInterface; /** * Configuration class for ordered items @@ -68,23 +70,30 @@ abstract class Ordered extends \Magento\Framework\App\Config\Base * @var \Magento\Sales\Model\Config */ protected $_salesConfig; + /** + * @var SerializerInterface + */ + private $serializer; /** * @param \Magento\Framework\App\Cache\Type\Config $configCacheType * @param \Psr\Log\LoggerInterface $logger * @param \Magento\Sales\Model\Config $salesConfig * @param \Magento\Framework\Simplexml\Element $sourceData + * @param SerializerInterface $serializer */ public function __construct( \Magento\Framework\App\Cache\Type\Config $configCacheType, \Psr\Log\LoggerInterface $logger, \Magento\Sales\Model\Config $salesConfig, - $sourceData = null + $sourceData = null, + SerializerInterface $serializer = null ) { parent::__construct($sourceData); $this->_configCacheType = $configCacheType; $this->_logger = $logger; $this->_salesConfig = $salesConfig; + $this->serializer = $serializer ?: ObjectManager::getInstance()->get(SerializerInterface::class); } /** @@ -179,11 +188,11 @@ abstract class Ordered extends \Magento\Framework\App\Config\Base $sortedCodes = []; $cachedData = $this->_configCacheType->load($this->_collectorsCacheKey); if ($cachedData) { - $sortedCodes = unserialize($cachedData); + $sortedCodes = $this->serializer->unserialize($cachedData); } if (!$sortedCodes) { $sortedCodes = $this->_getSortedCollectorCodes($this->_modelsConfig); - $this->_configCacheType->save(serialize($sortedCodes), $this->_collectorsCacheKey); + $this->_configCacheType->save($this->serializer->serialize($sortedCodes), $this->_collectorsCacheKey); } foreach ($sortedCodes as $code) { $this->_collectors[$code] = $this->_models[$code]; diff --git a/app/code/Magento/Sales/Model/Order/Total/Config/Base.php b/app/code/Magento/Sales/Model/Order/Total/Config/Base.php index 22c71f48b6f..20647e34c45 100644 --- a/app/code/Magento/Sales/Model/Order/Total/Config/Base.php +++ b/app/code/Magento/Sales/Model/Order/Total/Config/Base.php @@ -4,6 +4,7 @@ * See COPYING.txt for license details. */ namespace Magento\Sales\Model\Order\Total\Config; +use Magento\Framework\Serialize\SerializerInterface; /** * Configuration class for totals @@ -42,15 +43,17 @@ class Base extends \Magento\Sales\Model\Config\Ordered * @param \Magento\Sales\Model\Config $salesConfig * @param \Magento\Sales\Model\Order\TotalFactory $orderTotalFactory * @param mixed $sourceData + * @param SerializerInterface $serializer */ public function __construct( \Magento\Framework\App\Cache\Type\Config $configCacheType, \Psr\Log\LoggerInterface $logger, \Magento\Sales\Model\Config $salesConfig, \Magento\Sales\Model\Order\TotalFactory $orderTotalFactory, - $sourceData = null + $sourceData = null, + SerializerInterface $serializer = null ) { - parent::__construct($configCacheType, $logger, $salesConfig, $sourceData); + parent::__construct($configCacheType, $logger, $salesConfig, $sourceData, $serializer); $this->_orderTotalFactory = $orderTotalFactory; } diff --git a/app/code/Magento/Sales/Test/Unit/Model/Order/Total/Config/BaseTest.php b/app/code/Magento/Sales/Test/Unit/Model/Order/Total/Config/BaseTest.php index ed44331f577..bd519e76585 100644 --- a/app/code/Magento/Sales/Test/Unit/Model/Order/Total/Config/BaseTest.php +++ b/app/code/Magento/Sales/Test/Unit/Model/Order/Total/Config/BaseTest.php @@ -5,24 +5,28 @@ */ namespace Magento\Sales\Test\Unit\Model\Order\Total\Config; +use Magento\Framework\Serialize\SerializerInterface; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; class BaseTest extends \PHPUnit_Framework_TestCase { /** @var \Magento\Sales\Model\Order\Total\Config\Base */ - protected $object; + private $object; + + /** @var SerializerInterface|\PHPUnit_Framework_MockObject_MockObject */ + private $serializer; /** @var \Magento\Framework\App\Cache\Type\Config|\PHPUnit_Framework_MockObject_MockObject */ - protected $configCacheType; + private $configCacheType; /** @var \Psr\Log\LoggerInterface|\PHPUnit_Framework_MockObject_MockObject */ - protected $logger; + private $logger; /** @var \Magento\Sales\Model\Config|\PHPUnit_Framework_MockObject_MockObject */ - protected $salesConfig; + private $salesConfig; /** @var \Magento\Sales\Model\Order\TotalFactory|\PHPUnit_Framework_MockObject_MockObject */ - protected $orderTotalFactory; + private $orderTotalFactory; protected function setUp() { @@ -30,6 +34,7 @@ class BaseTest extends \PHPUnit_Framework_TestCase $this->logger = $this->getMock(\Psr\Log\LoggerInterface::class); $this->salesConfig = $this->getMock(\Magento\Sales\Model\Config::class, [], [], '', false); $this->orderTotalFactory = $this->getMock(\Magento\Sales\Model\Order\TotalFactory::class, [], [], '', false); + $this->serializer = $this->getMockForAbstractClass(SerializerInterface::class); $objectManager = new ObjectManager($this); $this->object = $objectManager->getObject( @@ -39,6 +44,7 @@ class BaseTest extends \PHPUnit_Framework_TestCase 'logger' => $this->logger, 'salesConfig' => $this->salesConfig, 'orderTotalFactory' => $this->orderTotalFactory, + 'serializer' => $this->serializer, ] ); } @@ -59,8 +65,14 @@ class BaseTest extends \PHPUnit_Framework_TestCase ->with(\Magento\Sales\Model\Order\Total\AbstractTotal::class) ->will($this->returnValue($total)); + $sortedCodes = ['other_code', 'some_code']; + $serializedCodes = '["other_code", "some_code"]'; + $this->serializer->expects($this->once()) + ->method('serialize') + ->with($sortedCodes) + ->willReturn($serializedCodes); $this->configCacheType->expects($this->once())->method('save') - ->with('a:2:{i:0;s:10:"other_code";i:1;s:9:"some_code";}', 'sorted_collectors'); + ->with($serializedCodes, 'sorted_collectors'); $this->assertSame( ['other_code' => $total, 'some_code' => $total], @@ -106,8 +118,14 @@ class BaseTest extends \PHPUnit_Framework_TestCase ->with(\Magento\Sales\Model\Order\Total\AbstractTotal::class) ->will($this->returnValue($total)); + $sortedCodes = ['other_code', 'some_code']; + $serializedCodes = '["other_code", "some_code"]'; $this->configCacheType->expects($this->once())->method('load')->with('sorted_collectors') - ->will($this->returnValue('a:2:{i:0;s:10:"other_code";i:1;s:9:"some_code";}')); + ->will($this->returnValue($serializedCodes)); + $this->serializer->expects($this->once()) + ->method('unserialize') + ->with($serializedCodes) + ->willReturn($sortedCodes); $this->configCacheType->expects($this->never())->method('save'); $this->assertSame( -- GitLab