Skip to content

Commit

Permalink
Merge pull request #792 from magento-troll/MAGETWO-58456
Browse files Browse the repository at this point in the history
MAGETWO-58456: Remove uses of unserialize in \Magento\Rule\Model\AbstractModel and its child classes and their usages
  • Loading branch information
rganin authored Feb 1, 2017
2 parents ca6ac76 + fba9a82 commit 5d1e030
Show file tree
Hide file tree
Showing 23 changed files with 893 additions and 76 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
*/
namespace Magento\CatalogRule\Model\ResourceModel\Rule;

use Magento\Framework\Serialize\Serializer\Json;
use Magento\Framework\App\ObjectManager;

class Collection extends \Magento\Rule\Model\ResourceModel\Rule\Collection\AbstractCollection
{
/**
Expand All @@ -14,6 +17,11 @@ class Collection extends \Magento\Rule\Model\ResourceModel\Rule\Collection\Abstr
*/
protected $_associatedEntitiesMap;

/**
* @var Json
*/
protected $serializer;

/**
* Collection constructor.
* @param \Magento\Framework\Data\Collection\EntityFactoryInterface $entityFactory
Expand All @@ -22,17 +30,20 @@ class Collection extends \Magento\Rule\Model\ResourceModel\Rule\Collection\Abstr
* @param \Magento\Framework\Event\ManagerInterface $eventManager
* @param \Magento\Framework\DB\Adapter\AdapterInterface $connection
* @param \Magento\Framework\Model\ResourceModel\Db\AbstractDb $resource
* @param Json|null $serializer
*/
public function __construct(
\Magento\Framework\Data\Collection\EntityFactoryInterface $entityFactory,
\Psr\Log\LoggerInterface $logger,
\Magento\Framework\Data\Collection\Db\FetchStrategyInterface $fetchStrategy,
\Magento\Framework\Event\ManagerInterface $eventManager,
\Magento\Framework\DB\Adapter\AdapterInterface $connection = null,
\Magento\Framework\Model\ResourceModel\Db\AbstractDb $resource = null
\Magento\Framework\Model\ResourceModel\Db\AbstractDb $resource = null,
Json $serializer = null
) {
parent::__construct($entityFactory, $logger, $fetchStrategy, $eventManager, $connection, $resource);
$this->_associatedEntitiesMap = $this->getAssociatedEntitiesMap();
$this->serializer = $serializer ?: ObjectManager::getInstance()->get(Json::class);
}

/**
Expand All @@ -55,7 +66,7 @@ protected function _construct()
*/
public function addAttributeInConditionFilter($attributeCode)
{
$match = sprintf('%%%s%%', substr(serialize(['attribute' => $attributeCode]), 5, -1));
$match = sprintf('%%%s%%', substr($this->serializer->serialize(['attribute' => $attributeCode]), 1, -1));
$this->addFieldToFilter('conditions_serialized', ['like' => $match]);

return $this;
Expand Down Expand Up @@ -96,7 +107,6 @@ protected function mapAssociatedEntities($entityType, $objectField)

/**
* @return $this
* @throws \Exception
*/
protected function _afterLoad()
{
Expand Down
7 changes: 5 additions & 2 deletions app/code/Magento/CatalogRule/Model/Rule.php
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ class Rule extends \Magento\Rule\Model\AbstractModel implements RuleInterface, I
* @param array $data
* @param ExtensionAttributesFactory|null $extensionFactory
* @param AttributeValueFactory|null $customAttributeFactory
* @param \Magento\Framework\Serialize\Serializer\Json $serializer
*
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
*/
Expand All @@ -186,7 +187,8 @@ public function __construct(
array $relatedCacheTypes = [],
array $data = [],
ExtensionAttributesFactory $extensionFactory = null,
AttributeValueFactory $customAttributeFactory = null
AttributeValueFactory $customAttributeFactory = null,
\Magento\Framework\Serialize\Serializer\Json $serializer = null
) {
$this->_productCollectionFactory = $productCollectionFactory;
$this->_storeManager = $storeManager;
Expand All @@ -210,7 +212,8 @@ public function __construct(
$resourceCollection,
$data,
$extensionFactory,
$customAttributeFactory
$customAttributeFactory,
$serializer
);
}

Expand Down
82 changes: 82 additions & 0 deletions app/code/Magento/CatalogRule/Setup/UpgradeData.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php
/**
* Copyright © 2013-2017 Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

namespace Magento\CatalogRule\Setup;

use Magento\Framework\DB\FieldDataConverterFactory;
use Magento\Framework\DB\DataConverter\SerializedToJson;
use Magento\Framework\Setup\ModuleContextInterface;
use Magento\Framework\Setup\ModuleDataSetupInterface;
use Magento\Framework\Setup\UpgradeDataInterface;
use Magento\Framework\EntityManager\MetadataPool;
use Magento\CatalogRule\Api\Data\RuleInterface;

class UpgradeData implements UpgradeDataInterface
{
/**
* @var FieldDataConverterFactory
*/
private $fieldDataConverterFactory;

/**
* @var MetadataPool
*/
private $metadataPool;

/**
* UpgradeData constructor.
*
* @param FieldDataConverterFactory $fieldDataConverterFactory
* @param MetadataPool $metadataPool
*/
public function __construct(
FieldDataConverterFactory $fieldDataConverterFactory,
MetadataPool $metadataPool
) {
$this->fieldDataConverterFactory = $fieldDataConverterFactory;
$this->metadataPool = $metadataPool;
}

/**
* @inheritdoc
*/
public function upgrade(ModuleDataSetupInterface $setup, ModuleContextInterface $context)
{
$setup->startSetup();

if (version_compare($context->getVersion(), '2.0.3', '<')) {
$this->convertSerializedDataToJson($setup);
}

$setup->endSetup();
}

/**
* Convert metadata from serialized to JSON format:
*
* @param ModuleDataSetupInterface $setup
*
* @return void
*/
public function convertSerializedDataToJson($setup)
{
$fieldDataConverter = $this->fieldDataConverterFactory->create(SerializedToJson::class);
$metadata = $this->metadataPool->getMetadata(RuleInterface::class);

$fieldDataConverter->convert(
$setup->getConnection(),
$setup->getTable('catalogrule'),
$metadata->getLinkField(),
'conditions_serialized'
);
$fieldDataConverter->convert(
$setup->getConnection(),
$setup->getTable('catalogrule'),
$metadata->getLinkField(),
'actions_serialized'
);
}
}
36 changes: 36 additions & 0 deletions app/code/Magento/CatalogRule/Test/Unit/Model/RuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,46 @@ protected function setUp()
'resourceIterator' => $this->_resourceIterator,
'extensionFactory' => $extensionFactoryMock,
'customAttributeFactory' => $attributeValueFactoryMock,
'serializer' => $this->getSerializerMock(),
]
);
}

/**
* Get mock for serializer
*
* @return \PHPUnit_Framework_MockObject_MockObject
*/
private function getSerializerMock()
{
$serializerMock = $this->getMockBuilder(\Magento\Framework\Serialize\Serializer\Json::class)
->disableOriginalConstructor()
->setMethods(['serialize', 'unserialize'])
->getMock();

$serializerMock->expects($this->any())
->method('serialize')
->will(
$this->returnCallback(
function ($value) {
return json_encode($value);
}
)
);

$serializerMock->expects($this->any())
->method('unserialize')
->will(
$this->returnCallback(
function ($value) {
return json_decode($value, true);
}
)
);

return $serializerMock;
}

/**
* @dataProvider dataProviderCallbackValidateProduct
* @param bool $validate
Expand Down
2 changes: 1 addition & 1 deletion app/code/Magento/CatalogRule/etc/module.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/
-->
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
<module name="Magento_CatalogRule" setup_version="2.0.2">
<module name="Magento_CatalogRule" setup_version="2.0.3">
<sequence>
<module name="Magento_Rule"/>
<module name="Magento_Catalog"/>
Expand Down
7 changes: 5 additions & 2 deletions app/code/Magento/CatalogWidget/Model/Rule.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class Rule extends \Magento\Rule\Model\AbstractModel
* @param ExtensionAttributesFactory|null $extensionFactory
* @param AttributeValueFactory|null $customAttributeFactory
*
* @param \Magento\Framework\Serialize\Serializer\Json $serializer
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
*/
public function __construct(
Expand All @@ -44,7 +45,8 @@ public function __construct(
\Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
array $data = [],
ExtensionAttributesFactory $extensionFactory = null,
AttributeValueFactory $customAttributeFactory = null
AttributeValueFactory $customAttributeFactory = null,
\Magento\Framework\Serialize\Serializer\Json $serializer = null
) {
$this->conditionsFactory = $conditionsFactory;
parent::__construct(
Expand All @@ -56,7 +58,8 @@ public function __construct(
$resourceCollection,
$data,
$extensionFactory,
$customAttributeFactory
$customAttributeFactory,
$serializer
);
}

Expand Down
21 changes: 16 additions & 5 deletions app/code/Magento/Rule/Model/AbstractModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ abstract class AbstractModel extends \Magento\Framework\Model\AbstractExtensible
*/
protected $_isReadonly = false;

/**
* @var \Magento\Framework\Serialize\Serializer\Json
*/
protected $serializer;

/**
* Getter for rule combine conditions instance
*
Expand Down Expand Up @@ -89,6 +94,8 @@ abstract public function getActionsInstance();
* @param array $data
* @param ExtensionAttributesFactory|null $extensionFactory
* @param AttributeValueFactory|null $customAttributeFactory
* @param \Magento\Framework\Serialize\Serializer\Json $serializer
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
*/
public function __construct(
\Magento\Framework\Model\Context $context,
Expand All @@ -99,10 +106,14 @@ public function __construct(
\Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
array $data = [],
ExtensionAttributesFactory $extensionFactory = null,
AttributeValueFactory $customAttributeFactory = null
AttributeValueFactory $customAttributeFactory = null,
\Magento\Framework\Serialize\Serializer\Json $serializer = null
) {
$this->_formFactory = $formFactory;
$this->_localeDate = $localeDate;
$this->serializer = $serializer ?: \Magento\Framework\App\ObjectManager::getInstance()->get(
\Magento\Framework\Serialize\Serializer\Json::class
);
parent::__construct(
$context,
$registry,
Expand Down Expand Up @@ -132,13 +143,13 @@ public function beforeSave()

// Serialize conditions
if ($this->getConditions()) {
$this->setConditionsSerialized(serialize($this->getConditions()->asArray()));
$this->setConditionsSerialized($this->serializer->serialize($this->getConditions()->asArray()));
$this->_conditions = null;
}

// Serialize actions
if ($this->getActions()) {
$this->setActionsSerialized(serialize($this->getActions()->asArray()));
$this->setActionsSerialized($this->serializer->serialize($this->getActions()->asArray()));
$this->_actions = null;
}

Expand Down Expand Up @@ -195,7 +206,7 @@ public function getConditions()
if ($this->hasConditionsSerialized()) {
$conditions = $this->getConditionsSerialized();
if (!empty($conditions)) {
$conditions = unserialize($conditions);
$conditions = $this->serializer->unserialize($conditions);
if (is_array($conditions) && !empty($conditions)) {
$this->_conditions->loadArray($conditions);
}
Expand Down Expand Up @@ -233,7 +244,7 @@ public function getActions()
if ($this->hasActionsSerialized()) {
$actions = $this->getActionsSerialized();
if (!empty($actions)) {
$actions = unserialize($actions);
$actions = $this->serializer->unserialize($actions);
if (is_array($actions) && !empty($actions)) {
$this->_actions->loadArray($actions);
}
Expand Down
Loading

0 comments on commit 5d1e030

Please sign in to comment.