From 02f80ea614775126960cd531381dfbc7880e7f21 Mon Sep 17 00:00:00 2001 From: Roman Ganin Date: Mon, 23 Jan 2017 14:14:03 +0200 Subject: [PATCH 01/19] MAGETWO-63479: Remove use of unserialize in Abstract --- app/code/Magento/CatalogRule/Model/Rule.php | 7 +- .../CatalogRule/Test/Unit/Model/RuleTest.php | 36 +++ app/code/Magento/CatalogWidget/Model/Rule.php | 7 +- app/code/Magento/Rule/Model/AbstractModel.php | 20 +- .../Test/Unit/Model/AbstractModelTest.php | 207 ++++++++++++++++++ app/code/Magento/SalesRule/Model/Rule.php | 7 +- 6 files changed, 273 insertions(+), 11 deletions(-) create mode 100644 app/code/Magento/Rule/Test/Unit/Model/AbstractModelTest.php diff --git a/app/code/Magento/CatalogRule/Model/Rule.php b/app/code/Magento/CatalogRule/Model/Rule.php index cf8ba21c2a4f6..11fd946344ae4 100644 --- a/app/code/Magento/CatalogRule/Model/Rule.php +++ b/app/code/Magento/CatalogRule/Model/Rule.php @@ -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) */ @@ -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; @@ -210,7 +212,8 @@ public function __construct( $resourceCollection, $data, $extensionFactory, - $customAttributeFactory + $customAttributeFactory, + $serializer ); } diff --git a/app/code/Magento/CatalogRule/Test/Unit/Model/RuleTest.php b/app/code/Magento/CatalogRule/Test/Unit/Model/RuleTest.php index a8a6326a5be67..49cef780dcbca 100644 --- a/app/code/Magento/CatalogRule/Test/Unit/Model/RuleTest.php +++ b/app/code/Magento/CatalogRule/Test/Unit/Model/RuleTest.php @@ -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 diff --git a/app/code/Magento/CatalogWidget/Model/Rule.php b/app/code/Magento/CatalogWidget/Model/Rule.php index 0871630721add..04eb0b072ae2a 100644 --- a/app/code/Magento/CatalogWidget/Model/Rule.php +++ b/app/code/Magento/CatalogWidget/Model/Rule.php @@ -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( @@ -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( @@ -56,7 +58,8 @@ public function __construct( $resourceCollection, $data, $extensionFactory, - $customAttributeFactory + $customAttributeFactory, + $serializer ); } diff --git a/app/code/Magento/Rule/Model/AbstractModel.php b/app/code/Magento/Rule/Model/AbstractModel.php index a324868376b4c..668a17b3cd703 100644 --- a/app/code/Magento/Rule/Model/AbstractModel.php +++ b/app/code/Magento/Rule/Model/AbstractModel.php @@ -49,6 +49,11 @@ abstract class AbstractModel extends \Magento\Framework\Model\AbstractExtensible */ protected $_isReadonly = false; + /** + * @var \Magento\Framework\Serialize\Serializer\Json + */ + private $serializer; + /** * Getter for rule combine conditions instance * @@ -89,6 +94,7 @@ abstract public function getActionsInstance(); * @param array $data * @param ExtensionAttributesFactory|null $extensionFactory * @param AttributeValueFactory|null $customAttributeFactory + * @param \Magento\Framework\Serialize\Serializer\Json $serializer */ public function __construct( \Magento\Framework\Model\Context $context, @@ -99,10 +105,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, @@ -132,13 +142,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; } @@ -195,7 +205,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); } @@ -233,7 +243,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); } diff --git a/app/code/Magento/Rule/Test/Unit/Model/AbstractModelTest.php b/app/code/Magento/Rule/Test/Unit/Model/AbstractModelTest.php new file mode 100644 index 0000000000000..81e63d65beeb7 --- /dev/null +++ b/app/code/Magento/Rule/Test/Unit/Model/AbstractModelTest.php @@ -0,0 +1,207 @@ +localeDateMock = $this->getMockBuilder(\Magento\Framework\Stdlib\DateTime\TimezoneInterface::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->formFactoryMock = $this->getMockBuilder(\Magento\Framework\Data\FormFactory::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->registryMock = $this->getMockBuilder(\Magento\Framework\Registry::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->contextMock = $this->getMockBuilder(\Magento\Framework\Model\Context::class) + ->disableOriginalConstructor() + ->setMethods(['getEventDispatcher']) + ->getMock(); + + $this->eventManagerMock = $this->getMockBuilder(\Magento\Framework\Event\ManagerInterface::class) + ->disableOriginalConstructor() + ->setMethods(['dispatch']) + ->getMock(); + $this->contextMock->expects($this->any()) + ->method('getEventDispatcher') + ->will($this->returnValue($this->eventManagerMock)); + + $resourceMock = $this->getMockBuilder(\Magento\Framework\Model\ResourceModel\AbstractResource::class) + ->disableOriginalConstructor() + ->getMock(); + $resourceCollectionMock = $this->getMockBuilder(\Magento\Framework\Data\Collection\AbstractDb::class) + ->disableOriginalConstructor() + ->getMock(); + $extensionFactory = $this->getMockBuilder(\Magento\Framework\Api\ExtensionAttributesFactory::class) + ->disableOriginalConstructor() + ->getMock(); + $customAttributeFactory = $this->getMockBuilder(\Magento\Framework\Api\AttributeValueFactory::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->model = $this->getMockForAbstractClass( + \Magento\Rule\Model\AbstractModel::class, + [ + 'context' => $this->contextMock, + 'registry' => $this->registryMock, + 'formFactory' => $this->formFactoryMock, + 'localeDate' => $this->localeDateMock, + 'resource' => $resourceMock, + 'resourceCollection' => $resourceCollectionMock, + 'data' => [], + 'extensionFactory' => $extensionFactory, + 'customAttributeFactory' => $customAttributeFactory, + 'serializer' => $this->getSerializerMock(), + ] + ); + } + + /** + * Get mock for serializer + * + * @return \Magento\Framework\Serialize\Serializer\Json|\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; + } + + public function testGetConditions() + { + $conditionsArray = ['conditions' => 'serialized']; + $serializedConditions = json_encode($conditionsArray); + $conditions = $this->getMockBuilder(\Magento\Rule\Model\Condition\Combine::class) + ->setMethods(['setRule', 'setId', 'setPrefix', 'loadArray']) + ->disableOriginalConstructor() + ->getMock(); + + $conditions->expects($this->once())->method('setRule')->will($this->returnSelf()); + $conditions->expects($this->once())->method('setId')->will($this->returnSelf()); + $conditions->expects($this->once())->method('setPrefix')->will($this->returnSelf()); + + $this->model->expects($this->once())->method('getConditionsInstance')->will($this->returnValue($conditions)); + + $this->model->setConditionsSerialized($serializedConditions); + + $conditions->expects($this->once())->method('loadArray')->with($conditionsArray); + + $this->assertEquals($conditions, $this->model->getConditions()); + } + + public function testGetActions() + { + $actionsArray = ['actions' => 'some_actions']; + $actionsSerialized = json_encode($actionsArray); + $actions = $this->getMockBuilder(\Magento\Rule\Model\Action\Collection::class) + ->setMethods(['setRule', 'setId', 'setPrefix', 'loadArray']) + ->disableOriginalConstructor() + ->getMock(); + + $actions->expects($this->once())->method('setRule')->will($this->returnSelf()); + $actions->expects($this->once())->method('setId')->will($this->returnSelf()); + $actions->expects($this->once())->method('setPrefix')->will($this->returnSelf()); + + $this->model->expects($this->once())->method('getActionsInstance')->will($this->returnValue($actions)); + + $this->model->setActionsSerialized($actionsSerialized); + + $actions->expects($this->once())->method('loadArray')->with($actionsArray); + + $this->assertEquals($actions, $this->model->getActions()); + } + + public function testBeforeSave() + { + $conditions = $this->getMockBuilder(\Magento\Rule\Model\Condition\Combine::class) + ->setMethods(['asArray']) + ->disableOriginalConstructor() + ->getMock(); + + $actions = $this->getMockBuilder(\Magento\Rule\Model\Action\Collection::class) + ->setMethods(['asArray']) + ->disableOriginalConstructor() + ->getMock(); + + $this->model->setConditions($conditions); + $this->model->setActions($actions); + + $conditions->expects($this->any())->method('asArray')->will($this->returnValue(['conditions' => 'array'])); + $actions->expects($this->any())->method('asArray')->will($this->returnValue(['actions' => 'array'])); + + $this->eventManagerMock->expects($this->exactly(2))->method('dispatch'); + + $this->assertEquals($this->model, $this->model->beforeSave()); + $this->assertEquals(json_encode(['conditions' => 'array']), $this->model->getConditionsSerialized()); + $this->assertEquals(json_encode(['actions' => 'array']), $this->model->getActionsSerialized()); + } +} diff --git a/app/code/Magento/SalesRule/Model/Rule.php b/app/code/Magento/SalesRule/Model/Rule.php index 75039377d2b85..10f15ab4edc83 100644 --- a/app/code/Magento/SalesRule/Model/Rule.php +++ b/app/code/Magento/SalesRule/Model/Rule.php @@ -190,6 +190,7 @@ class Rule extends \Magento\Rule\Model\AbstractModel * @param array $data * @param ExtensionAttributesFactory|null $extensionFactory * @param AttributeValueFactory|null $customAttributeFactory + * @param \Magento\Framework\Serialize\Serializer\Json $serializer * * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ @@ -208,7 +209,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->_couponFactory = $couponFactory; $this->_codegenFactory = $codegenFactory; @@ -225,7 +227,8 @@ public function __construct( $resourceCollection, $data, $extensionFactory, - $customAttributeFactory + $customAttributeFactory, + $serializer ); } From 46949f7c0085612937f8fd1f7c87ffe417f7263c Mon Sep 17 00:00:00 2001 From: Roman Ganin Date: Mon, 23 Jan 2017 15:58:55 +0200 Subject: [PATCH 02/19] MAGETWO-63479: Remove use of unserialize in Abstract --- app/code/Magento/Rule/Model/AbstractModel.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Rule/Model/AbstractModel.php b/app/code/Magento/Rule/Model/AbstractModel.php index 668a17b3cd703..618f3b08ea4dc 100644 --- a/app/code/Magento/Rule/Model/AbstractModel.php +++ b/app/code/Magento/Rule/Model/AbstractModel.php @@ -52,7 +52,7 @@ abstract class AbstractModel extends \Magento\Framework\Model\AbstractExtensible /** * @var \Magento\Framework\Serialize\Serializer\Json */ - private $serializer; + protected $serializer; /** * Getter for rule combine conditions instance From bb9a4f36739289ef956b9d8255bb41ddd59bdfaf Mon Sep 17 00:00:00 2001 From: Stanislav Lopukhov Date: Mon, 23 Jan 2017 18:24:14 +0200 Subject: [PATCH 03/19] MAGETWO-63344: Remove use of serialize in \Magento\CatalogRule\Model\Rule for catalogrule table --- .../Model/ResourceModel/Rule/Collection.php | 15 +++- .../ResourceModel/Rule/CollectionTest.php | 82 +++++++++++++++++++ 2 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 dev/tests/integration/testsuite/Magento/CatalogRule/Model/ResourceModel/Rule/CollectionTest.php diff --git a/app/code/Magento/CatalogRule/Model/ResourceModel/Rule/Collection.php b/app/code/Magento/CatalogRule/Model/ResourceModel/Rule/Collection.php index 67feaa71c6b5a..d483420b90bd7 100644 --- a/app/code/Magento/CatalogRule/Model/ResourceModel/Rule/Collection.php +++ b/app/code/Magento/CatalogRule/Model/ResourceModel/Rule/Collection.php @@ -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 { /** @@ -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 @@ -22,6 +30,7 @@ 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, @@ -29,10 +38,12 @@ public function __construct( \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); } /** @@ -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; diff --git a/dev/tests/integration/testsuite/Magento/CatalogRule/Model/ResourceModel/Rule/CollectionTest.php b/dev/tests/integration/testsuite/Magento/CatalogRule/Model/ResourceModel/Rule/CollectionTest.php new file mode 100644 index 0000000000000..457e267f3dd34 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/CatalogRule/Model/ResourceModel/Rule/CollectionTest.php @@ -0,0 +1,82 @@ +objectManager = Bootstrap::getObjectManager(); + $this->indexBuilder = $this->objectManager->get(IndexBuilder::class); + $this->resourceRule = $this->objectManager->get(Rule::class); + $this->resourceRuleCollection = $this->objectManager->get(Collection::class); + } + + /** + * @magentoAppArea adminhtml + * + * @magentoDataFixture Magento/CatalogRule/_files/attribute.php + * @magentoDataFixture Magento/CatalogRule/_files/rule_by_attribute.php + * @magentoDataFixture Magento/CatalogRule/_files/two_rules.php + */ + public function testReindexAfterRuleCreation() + { + $this->indexBuilder->reindexFull(); + $installer = $this->objectManager->create(CategorySetup::class); + + $resourceRuleCollection = $this->objectManager->create(RuleCollection::class); + $resourceRuleCollection->addFilter('is_active', 1); + $this->assertEquals(3, $resourceRuleCollection->count()); + + $resourceRuleCollection = $this->objectManager->create(RuleCollection::class); + $resourceRuleCollection->addFilter('is_active', 1); + $resourceRuleCollection->addFilter('name', 'test_rule'); + $this->assertEquals(1, $resourceRuleCollection->count()); + + $model = $this->objectManager->create(Attribute::class); + $model->loadByCode($installer->getEntityTypeId('catalog_product'), 'test_attribute'); + $model->delete(); + + $resourceRuleCollection = $this->objectManager->create(RuleCollection::class); + $resourceRuleCollection->addFilter('is_active', 1); + $this->assertEquals(2, $resourceRuleCollection->count()); + $resourceRuleCollection = $this->objectManager->create(RuleCollection::class); + $resourceRuleCollection->addFilter('is_active', 1); + $resourceRuleCollection->addFilter('name', 'test_rule'); + $this->assertEquals(0, $resourceRuleCollection->count()); + } +} From 6962f95230e4509bc86cb08d88454ad1f00bf764 Mon Sep 17 00:00:00 2001 From: Stanislav Lopukhov Date: Mon, 23 Jan 2017 18:27:46 +0200 Subject: [PATCH 04/19] MAGETWO-63344: Remove use of serialize in \Magento\CatalogRule\Model\Rule for catalogrule table --- .../CatalogRule/Model/ResourceModel/Rule/CollectionTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/dev/tests/integration/testsuite/Magento/CatalogRule/Model/ResourceModel/Rule/CollectionTest.php b/dev/tests/integration/testsuite/Magento/CatalogRule/Model/ResourceModel/Rule/CollectionTest.php index 457e267f3dd34..156f206408dc8 100644 --- a/dev/tests/integration/testsuite/Magento/CatalogRule/Model/ResourceModel/Rule/CollectionTest.php +++ b/dev/tests/integration/testsuite/Magento/CatalogRule/Model/ResourceModel/Rule/CollectionTest.php @@ -74,6 +74,7 @@ public function testReindexAfterRuleCreation() $resourceRuleCollection = $this->objectManager->create(RuleCollection::class); $resourceRuleCollection->addFilter('is_active', 1); $this->assertEquals(2, $resourceRuleCollection->count()); + $resourceRuleCollection = $this->objectManager->create(RuleCollection::class); $resourceRuleCollection->addFilter('is_active', 1); $resourceRuleCollection->addFilter('name', 'test_rule'); From 408aa0c32ecd7fd2cdfe172a705b67e076bf9ab9 Mon Sep 17 00:00:00 2001 From: Stanislav Lopukhov Date: Tue, 24 Jan 2017 12:39:41 +0200 Subject: [PATCH 05/19] MAGETWO-63344: Remove use of serialize in \Magento\CatalogRule\Model\Rule for catalogrule table --- .../Magento/CatalogRule/Setup/UpgradeData.php | 70 +++++++++++++++++++ app/code/Magento/CatalogRule/etc/module.xml | 2 +- 2 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 app/code/Magento/CatalogRule/Setup/UpgradeData.php diff --git a/app/code/Magento/CatalogRule/Setup/UpgradeData.php b/app/code/Magento/CatalogRule/Setup/UpgradeData.php new file mode 100644 index 0000000000000..35f5b3b1c904c --- /dev/null +++ b/app/code/Magento/CatalogRule/Setup/UpgradeData.php @@ -0,0 +1,70 @@ +fieldDataConverterFactory = $fieldDataConverterFactory; + } + + /** + * @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); + $fieldDataConverter->convert( + $setup->getConnection(), + $setup->getTable('catalogrule'), + 'rule_id', + 'conditions_serialized' + ); + $fieldDataConverter->convert( + $setup->getConnection(), + $setup->getTable('catalogrule'), + 'rule_id', + 'actions_serialized' + ); + } +} diff --git a/app/code/Magento/CatalogRule/etc/module.xml b/app/code/Magento/CatalogRule/etc/module.xml index 0a9c185d87512..6d3a409fbbc85 100644 --- a/app/code/Magento/CatalogRule/etc/module.xml +++ b/app/code/Magento/CatalogRule/etc/module.xml @@ -6,7 +6,7 @@ */ --> - + From c1f408aec13214d56582c1fe5f39d46d0b97bf69 Mon Sep 17 00:00:00 2001 From: Stanislav Lopukhov Date: Wed, 25 Jan 2017 12:34:37 +0200 Subject: [PATCH 06/19] MAGETWO-63344: Remove use of serialize in \Magento\CatalogRule\Model\Rule for catalogrule table --- .../Magento/CatalogRule/Setup/UpgradeData.php | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/CatalogRule/Setup/UpgradeData.php b/app/code/Magento/CatalogRule/Setup/UpgradeData.php index 35f5b3b1c904c..7c34d5a3f54c6 100644 --- a/app/code/Magento/CatalogRule/Setup/UpgradeData.php +++ b/app/code/Magento/CatalogRule/Setup/UpgradeData.php @@ -11,6 +11,8 @@ 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 { @@ -20,14 +22,22 @@ class UpgradeData implements UpgradeDataInterface private $fieldDataConverterFactory; /** - * Constructor + * @var MetadataPool + */ + private $metadataPool; + + /** + * UpgradeData constructor. * * @param FieldDataConverterFactory $fieldDataConverterFactory + * @param MetadataPool $metadataPool */ public function __construct( - FieldDataConverterFactory $fieldDataConverterFactory + FieldDataConverterFactory $fieldDataConverterFactory, + MetadataPool $metadataPool ) { $this->fieldDataConverterFactory = $fieldDataConverterFactory; + $this->metadataPool = $metadataPool; } /** @@ -54,16 +64,18 @@ public function upgrade(ModuleDataSetupInterface $setup, ModuleContextInterface public function convertSerializedDataToJson($setup) { $fieldDataConverter = $this->fieldDataConverterFactory->create(SerializedToJson::class); + $metadata = $this->metadataPool->getMetadata(RuleInterface::class); + $fieldDataConverter->convert( $setup->getConnection(), $setup->getTable('catalogrule'), - 'rule_id', + $metadata->getLinkField(), 'conditions_serialized' ); $fieldDataConverter->convert( $setup->getConnection(), $setup->getTable('catalogrule'), - 'rule_id', + $metadata->getLinkField(), 'actions_serialized' ); } From d60f8ecbe0a76b5cbd44e3a8568edc08ebee32cd Mon Sep 17 00:00:00 2001 From: Stanislav Lopukhov Date: Wed, 25 Jan 2017 14:52:36 +0200 Subject: [PATCH 07/19] MAGETWO-63634: Update Field Data Converter --- .../Framework/DB/FieldDataConverter.php | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/lib/internal/Magento/Framework/DB/FieldDataConverter.php b/lib/internal/Magento/Framework/DB/FieldDataConverter.php index 2e6e3c20274c6..58e50f725ddd9 100644 --- a/lib/internal/Magento/Framework/DB/FieldDataConverter.php +++ b/lib/internal/Magento/Framework/DB/FieldDataConverter.php @@ -9,6 +9,7 @@ use Magento\Framework\DB\Query\Generator; use Magento\Framework\DB\DataConverter\DataConverterInterface; use Magento\Framework\DB\Select\QueryModifierInterface; +use Magento\Framework\DB\Select; /** * Convert field data from one representation to another @@ -40,16 +41,17 @@ public function __construct( } /** - * Convert field data from one representation to another + * Get converter select * * @param AdapterInterface $connection * @param string $table * @param string $identifier * @param string $field * @param QueryModifierInterface|null $queryModifier - * @return void + * + * @return Select */ - public function convert( + public function getSelect( AdapterInterface $connection, $table, $identifier, @@ -62,6 +64,28 @@ public function convert( if ($queryModifier) { $queryModifier->modify($select); } + + return $select; + } + + /** + * Convert field data from one representation to another + * + * @param AdapterInterface $connection + * @param string $table + * @param string $identifier + * @param string $field + * @param QueryModifierInterface|null $queryModifier + * @return void + */ + public function convert( + AdapterInterface $connection, + $table, + $identifier, + $field, + QueryModifierInterface $queryModifier = null + ) { + $select = $this->getSelect($connection, $table, $identifier, $field, $queryModifier); $iterator = $this->queryGenerator->generate($identifier, $select); foreach ($iterator as $selectByRange) { $rows = $connection->fetchAll($selectByRange); From edfcd672296cb8a855ac8b3817517fe211b2d3c0 Mon Sep 17 00:00:00 2001 From: Mykola Palamar Date: Wed, 25 Jan 2017 17:04:18 +0200 Subject: [PATCH 08/19] MAGETWO-63350: Remove use of unserialize in \Magento\SalesRule\Model\Rule for salesrule field --- .../SalesRule/Model/Converter/ToDataModel.php | 15 ++- .../SalesRule/Model/ResourceModel/Rule.php | 28 +++--- .../Model/ResourceModel/Rule/Collection.php | 13 ++- .../Magento/SalesRule/Setup/UpgradeData.php | 81 ++++++++++++++++ .../Unit/Model/Converter/ToDataModelTest.php | 92 +++++++++++++------ .../Unit/Model/ResourceModel/RuleTest.php | 58 ++++++++++++ app/code/Magento/SalesRule/etc/module.xml | 2 +- .../ResourceModel/Rule/CollectionTest.php | 33 +++++++ .../Model/ResourceModel/RuleTest.php | 27 ++++++ .../_files/rule_custom_product_attribute.php | 76 +++++++++++++++ 10 files changed, 377 insertions(+), 48 deletions(-) create mode 100644 app/code/Magento/SalesRule/Setup/UpgradeData.php create mode 100644 dev/tests/integration/testsuite/Magento/SalesRule/Model/ResourceModel/RuleTest.php create mode 100644 dev/tests/integration/testsuite/Magento/SalesRule/_files/rule_custom_product_attribute.php diff --git a/app/code/Magento/SalesRule/Model/Converter/ToDataModel.php b/app/code/Magento/SalesRule/Model/Converter/ToDataModel.php index 2305feaa46730..bfbce07eba7a2 100644 --- a/app/code/Magento/SalesRule/Model/Converter/ToDataModel.php +++ b/app/code/Magento/SalesRule/Model/Converter/ToDataModel.php @@ -9,6 +9,7 @@ use Magento\SalesRule\Api\Data\RuleInterface; use Magento\SalesRule\Model\Data\Rule as RuleDataModel; use Magento\SalesRule\Model\Rule; +use Magento\Framework\Serialize\Serializer\Json; class ToDataModel { @@ -37,25 +38,33 @@ class ToDataModel */ protected $ruleLabelFactory; + /** + * @var Json $serializer + */ + private $serializer; + /** * @param \Magento\SalesRule\Model\RuleFactory $ruleFactory * @param \Magento\SalesRule\Api\Data\RuleInterfaceFactory $ruleDataFactory * @param \Magento\SalesRule\Api\Data\ConditionInterfaceFactory $conditionDataFactory * @param \Magento\SalesRule\Api\Data\RuleLabelInterfaceFactory $ruleLabelFactory * @param \Magento\Framework\Reflection\DataObjectProcessor $dataObjectProcessor + * @param Json $serializer Optional parameter for backward compatibility */ public function __construct( \Magento\SalesRule\Model\RuleFactory $ruleFactory, \Magento\SalesRule\Api\Data\RuleInterfaceFactory $ruleDataFactory, \Magento\SalesRule\Api\Data\ConditionInterfaceFactory $conditionDataFactory, \Magento\SalesRule\Api\Data\RuleLabelInterfaceFactory $ruleLabelFactory, - \Magento\Framework\Reflection\DataObjectProcessor $dataObjectProcessor + \Magento\Framework\Reflection\DataObjectProcessor $dataObjectProcessor, + Json $serializer = null ) { $this->ruleFactory = $ruleFactory; $this->ruleDataFactory = $ruleDataFactory; $this->conditionDataFactory = $conditionDataFactory; $this->ruleLabelFactory = $ruleLabelFactory; $this->dataObjectProcessor = $dataObjectProcessor; + $this->serializer = $serializer ?: \Magento\Framework\App\ObjectManager::getInstance()->get(Json::class); } /** @@ -81,7 +90,7 @@ protected function mapConditions(RuleDataModel $dataModel, \Magento\SalesRule\Mo { $conditionSerialized = $ruleModel->getConditionsSerialized(); if ($conditionSerialized) { - $conditionArray = unserialize($conditionSerialized); + $conditionArray = $this->serializer->unserialize($conditionSerialized); $conditionDataModel = $this->arrayToConditionDataModel($conditionArray); $dataModel->setCondition($conditionDataModel); } else { @@ -99,7 +108,7 @@ protected function mapActionConditions(RuleDataModel $dataModel, \Magento\SalesR { $actionConditionSerialized = $ruleModel->getActionsSerialized(); if ($actionConditionSerialized) { - $actionConditionArray = unserialize($actionConditionSerialized); + $actionConditionArray = $this->serializer->unserialize($actionConditionSerialized); $actionConditionDataModel = $this->arrayToConditionDataModel($actionConditionArray); $dataModel->setActionCondition($actionConditionDataModel); } else { diff --git a/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php b/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php index 530a041236402..414891be018bf 100644 --- a/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php +++ b/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php @@ -10,6 +10,7 @@ use Magento\Framework\Model\AbstractModel; use Magento\Rule\Model\ResourceModel\AbstractResource; use Magento\Framework\EntityManager\EntityManager; +use Magento\Framework\Serialize\Serializer\Json; /** * Sales Rule resource model @@ -56,13 +57,15 @@ class Rule extends AbstractResource * @param \Magento\SalesRule\Model\ResourceModel\Coupon $resourceCoupon * @param string $connectionName * @param \Magento\Framework\DataObject|null $associatedEntityMapInstance + * @param Json $serializer Optional parameter for backward compatibility */ public function __construct( \Magento\Framework\Model\ResourceModel\Db\Context $context, \Magento\Framework\Stdlib\StringUtils $string, \Magento\SalesRule\Model\ResourceModel\Coupon $resourceCoupon, $connectionName = null, - \Magento\Framework\DataObject $associatedEntityMapInstance = null + \Magento\Framework\DataObject $associatedEntityMapInstance = null, + Json $serializer = null ) { $this->string = $string; $this->_resourceCoupon = $resourceCoupon; @@ -70,6 +73,8 @@ public function __construct( \Magento\SalesRule\Model\ResourceModel\Rule\AssociatedEntityMap::class ); $this->_associatedEntitiesMap = $associatedEntitiesMapInstance->getData(); + /* we already have serialized field in AbstractResource, so we don't have to declare it here */ + $this->serializer = $serializer ?: \Magento\Framework\App\ObjectManager::getInstance()->get(Json::class); parent::__construct($context, $connectionName); } @@ -157,8 +162,8 @@ protected function _afterSave(AbstractModel $object) // Save product attributes used in rule $ruleProductAttributes = array_merge( - $this->getProductAttributes(serialize($object->getConditions()->asArray())), - $this->getProductAttributes(serialize($object->getActions()->asArray())) + $this->getProductAttributes($this->serializer->serialize($object->getConditions()->asArray())), + $this->getProductAttributes($this->serializer->serialize($object->getActions()->asArray())) ); if (count($ruleProductAttributes)) { $this->setActualProductAttributes($object, $ruleProductAttributes); @@ -345,19 +350,14 @@ public function setActualProductAttributes($rule, $attributes) */ public function getProductAttributes($serializedString) { - $result = []; - if (preg_match_all( - '~s:32:"salesrule/rule_condition_product";s:9:"attribute";s:\d+:"(.*?)"~s', + // we need 4 backslashes to match 1 in regexp, see http://www.php.net/manual/en/regexp.reference.escape.php + preg_match_all( + '~"Magento\\\\\\\\SalesRule\\\\\\\\Model\\\\\\\\Rule\\\\\\\\Condition\\\\\\\\Product","attribute":"(.*?)"~', $serializedString, $matches - ) - ) { - foreach ($matches[1] as $attributeCode) { - $result[] = $attributeCode; - } - } - - return $result; + ); + // we always have $matches like [[],[]] + return array_values($matches[1]); } /** diff --git a/app/code/Magento/SalesRule/Model/ResourceModel/Rule/Collection.php b/app/code/Magento/SalesRule/Model/ResourceModel/Rule/Collection.php index b884a39a8bd8a..4a5d0f6e3e418 100644 --- a/app/code/Magento/SalesRule/Model/ResourceModel/Rule/Collection.php +++ b/app/code/Magento/SalesRule/Model/ResourceModel/Rule/Collection.php @@ -7,6 +7,7 @@ namespace Magento\SalesRule\Model\ResourceModel\Rule; use Magento\Quote\Model\Quote\Address; +use Magento\Framework\Serialize\Serializer\Json; /** * Sales Rules resource collection model. @@ -31,6 +32,11 @@ class Collection extends \Magento\Rule\Model\ResourceModel\Rule\Collection\Abstr */ protected $_date; + /** + * @var Json $serializer + */ + private $serializer; + /** * @param \Magento\Framework\Data\Collection\EntityFactory $entityFactory * @param \Psr\Log\LoggerInterface $logger @@ -39,6 +45,7 @@ class Collection extends \Magento\Rule\Model\ResourceModel\Rule\Collection\Abstr * @param \Magento\Framework\Stdlib\DateTime\TimezoneInterface $date * @param mixed $connection * @param \Magento\Framework\Model\ResourceModel\Db\AbstractDb $resource + * @param Json $serializer Optional parameter for backward compatibility */ public function __construct( \Magento\Framework\Data\Collection\EntityFactory $entityFactory, @@ -47,10 +54,12 @@ public function __construct( \Magento\Framework\Event\ManagerInterface $eventManager, \Magento\Framework\Stdlib\DateTime\TimezoneInterface $date, \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->_date = $date; + $this->serializer = $serializer ?: \Magento\Framework\App\ObjectManager::getInstance()->get(Json::class); $this->_associatedEntitiesMap = $this->getAssociatedEntitiesMap(); } @@ -272,7 +281,7 @@ public function _initSelect() */ public function addAttributeInConditionFilter($attributeCode) { - $match = sprintf('%%%s%%', substr(serialize(['attribute' => $attributeCode]), 5, -1)); + $match = sprintf('%%%s%%', substr($this->serializer->serialize(['attribute' => $attributeCode]), 1, -1)); $field = $this->_getMappedField('conditions_serialized'); $cCond = $this->_getConditionSql($field, ['like' => $match]); $field = $this->_getMappedField('actions_serialized'); diff --git a/app/code/Magento/SalesRule/Setup/UpgradeData.php b/app/code/Magento/SalesRule/Setup/UpgradeData.php new file mode 100644 index 0000000000000..52cf832beb772 --- /dev/null +++ b/app/code/Magento/SalesRule/Setup/UpgradeData.php @@ -0,0 +1,81 @@ +fieldDataConverterFactory = $fieldDataConverterFactory; + $this->metadataPool = $metadataPool; + } + + /** + * @inheritdoc + */ + public function upgrade(ModuleDataSetupInterface $setup, ModuleContextInterface $context) + { + $setup->startSetup(); + + if (version_compare($context->getVersion(), '2.0.1', '<')) { + $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('salesrule'), + $metadata->getLinkField(), + 'conditions_serialized' + ); + $fieldDataConverter->convert( + $setup->getConnection(), + $setup->getTable('salesrule'), + $metadata->getLinkField(), + 'actions_serialized' + ); + } +} \ No newline at end of file diff --git a/app/code/Magento/SalesRule/Test/Unit/Model/Converter/ToDataModelTest.php b/app/code/Magento/SalesRule/Test/Unit/Model/Converter/ToDataModelTest.php index 2de1075cc1a0e..7815058fdba81 100644 --- a/app/code/Magento/SalesRule/Test/Unit/Model/Converter/ToDataModelTest.php +++ b/app/code/Magento/SalesRule/Test/Unit/Model/Converter/ToDataModelTest.php @@ -45,6 +45,11 @@ class ToDataModelTest extends \PHPUnit_Framework_TestCase */ protected $model; + /** + * @var \Magento\Framework\Serialize\Serializer\Json + */ + protected $serializer; + protected function setUp() { $this->ruleFactory = $this->getMockBuilder(\Magento\SalesRule\Model\RuleFactory::class) @@ -78,6 +83,10 @@ protected function setUp() ->setMethods(['_construct', 'getData', 'getConditionsSerialized', 'getActionsSerialized']) ->getMock(); + $this->serializer = $this->getMockBuilder(\Magento\Framework\Serialize\Serializer\Json::class) + ->setMethods(null) + ->getMock(); + $helper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); $this->model = $helper->getObject( \Magento\SalesRule\Model\Converter\ToDataModel::class, @@ -87,28 +96,51 @@ protected function setUp() 'conditionDataFactory' => $this->conditionDataFactory, 'ruleLabelFactory' => $this->ruleLabelFactory, 'dataObjectProcessor' => $this->dataObjectProcessor, + 'serializer' => $this->serializer, ] ); } - public function testToDataModel() + private function getArrayData() { - $array = [ + return [ 'rule_id' => '1', 'name' => 'testrule', 'is_active' => '1', - 'conditions_serialized' => - 'a:7:{s:4:"type";s:46:"Magento\SalesRule\Model\Rule\Condition\Combine";s:9:"attribute";N;' - . 's:8:"operator";N;s:5:"value";s:1:"1";s:18:"is_value_processed";N;s:10:"aggregator";s:3:"all";' - . 's:10:"conditions";a:1:{i:0;a:5:{s:4:"type";s:46:"Magento\SalesRule\Model\Rule\Condition\Address";' - . 's:9:"attribute";s:13:"base_subtotal";s:8:"operator";s:2:">=";s:5:"value";s:3:"100";' - . 's:18:"is_value_processed";b:0;}}}', - 'actions_serialized' => - 'a:7:{s:4:"type";s:54:"Magento\SalesRule\Model\Rule\Condition\Product\Combine";s:9:"attribute";N;' - . 's:8:"operator";N;s:5:"value";s:1:"1";s:18:"is_value_processed";N;s:10:"aggregator";s:3:"all";' - . 's:10:"conditions";a:1:{i:0;a:5:{s:4:"type";s:46:"Magento\SalesRule\Model\Rule\Condition\Product";' - . 's:9:"attribute";s:16:"attribute_set_id";s:8:"operator";s:2:"==";s:5:"value";s:1:"4";' - . 's:18:"is_value_processed";b:0;}}}', + 'conditions_serialized' => json_encode([ + 'type' => \Magento\SalesRule\Model\Rule\Condition\Combine::class, + 'attribute' => null, + 'operator' => null, + 'value' => '1', + 'is_value_processed' => null, + 'aggregator' => 'all', + 'conditions' => [ + [ + 'type' => \Magento\SalesRule\Model\Rule\Condition\Address::class, + 'attribute' => 'base_subtotal', + 'operator' => '>=', + 'value' => '100', + 'is_value_processed' => false, + ], + ], + ]), + 'actions_serialized' => json_encode([ + 'type' => \Magento\SalesRule\Model\Rule\Condition\Product\Combine::class, + 'attribute' => null, + 'operator' => null, + 'value' => '1', + 'is_value_processed' => null, + 'aggregator' => 'all', + 'conditions' => [ + [ + 'type' => \Magento\SalesRule\Model\Rule\Condition\Product::class, + 'attribute' => 'attribute_set_id', + 'operator' => '==', + 'value' => '4', + 'is_value_processed' => false, + ], + ], + ]), 'coupon_type' => '1', 'coupon_code' => '', 'store_labels' => [ @@ -116,11 +148,15 @@ public function testToDataModel() 1 => 'TestRuleForDefaultStore', ], ]; + } + public function testToDataModel() + { + $array = $this->getArrayData(); $dataModel = $this->getMockBuilder(\Magento\SalesRule\Model\Data\Rule::class) - ->disableOriginalConstructor() - ->setMethods(['create', 'getStoreLabels', 'setStoreLabels', 'getCouponType', 'setCouponType']) - ->getMock(); + ->disableOriginalConstructor() + ->setMethods(['create', 'getStoreLabels', 'setStoreLabels', 'getCouponType', 'setCouponType']) + ->getMock(); $dataLabel = $this->getMockBuilder(\Magento\SalesRule\Api\Data\RuleLabel::class) ->setMethods(['setStoreId', 'setStoreLabel', 'setStoreLabels']) @@ -158,9 +194,9 @@ public function testToDataModel() ->willReturn($array['conditions_serialized']); $dataModel - ->expects($this->atLeastOnce()) - ->method('getStoreLabels') - ->willReturn($array['store_labels']); + ->expects($this->atLeastOnce()) + ->method('getStoreLabels') + ->willReturn($array['store_labels']); $dataModel ->expects($this->atLeastOnce()) @@ -213,16 +249,16 @@ public function testArrayToConditionDataModel() 'is_value_processed' => null, 'aggregator' => 'all', 'conditions' => [ - [ - 'type' => \Magento\SalesRule\Model\Rule\Condition\Product::class, - 'attribute' => 'category_ids', - 'operator' => '==', - 'value' => 3, - 'is_value_processed' => null - ] - + [ + 'type' => \Magento\SalesRule\Model\Rule\Condition\Product::class, + 'attribute' => 'category_ids', + 'operator' => '==', + 'value' => 3, + 'is_value_processed' => null ] + ] + ], ] diff --git a/app/code/Magento/SalesRule/Test/Unit/Model/ResourceModel/RuleTest.php b/app/code/Magento/SalesRule/Test/Unit/Model/ResourceModel/RuleTest.php index a594ffe006c36..91aaf56a95c1b 100644 --- a/app/code/Magento/SalesRule/Test/Unit/Model/ResourceModel/RuleTest.php +++ b/app/code/Magento/SalesRule/Test/Unit/Model/ResourceModel/RuleTest.php @@ -186,4 +186,62 @@ public function testDelete() ->with($this->rule); $this->assertEquals($this->model->delete($this->rule), $this->model); } + + /** + * Check that can parse JSON string correctly. + * + * @param string $testString + * @param array $expects + * @dataProvider dataProviderForProductAttributes + */ + public function testGetProductAttributes($testString, $expects) + { + $result = $this->model->getProductAttributes($testString); + $this->assertEquals($expects, $result); + } + + /** + * @return array + */ + public function dataProviderForProductAttributes() + { + return [ + [ + json_encode([ + 'type' => 'Magento\SalesRule\Model\Rule\Condition\Product', + 'attribute' => 'some_attribute', + ]), + [ + 'some_attribute', + ] + ], + [ + json_encode([ + [ + 'type' => 'Magento\SalesRule\Model\Rule\Condition\Product', + 'attribute' => 'some_attribute', + ], + [ + 'type' => 'Magento\SalesRule\Model\Rule\Condition\Product', + 'attribute' => 'some_attribute2', + ], + ]), + [ + 'some_attribute', + 'some_attribute2', + ] + ], + [ + json_encode([ + 'type' => 'Magento\SalesRule\Model\Rule\Condition\Product\Found', + 'attribute' => 'some_attribute', + ]), + [] + ], + [ + json_encode([]), + [] + ], + ]; + } } diff --git a/app/code/Magento/SalesRule/etc/module.xml b/app/code/Magento/SalesRule/etc/module.xml index 464cb1382346e..db4095e37d471 100644 --- a/app/code/Magento/SalesRule/etc/module.xml +++ b/app/code/Magento/SalesRule/etc/module.xml @@ -6,7 +6,7 @@ */ --> - + diff --git a/dev/tests/integration/testsuite/Magento/SalesRule/Model/ResourceModel/Rule/CollectionTest.php b/dev/tests/integration/testsuite/Magento/SalesRule/Model/ResourceModel/Rule/CollectionTest.php index 6fce0da69c97b..00097b5523376 100644 --- a/dev/tests/integration/testsuite/Magento/SalesRule/Model/ResourceModel/Rule/CollectionTest.php +++ b/dev/tests/integration/testsuite/Magento/SalesRule/Model/ResourceModel/Rule/CollectionTest.php @@ -221,6 +221,39 @@ protected function setSpecificTimezone($timezone) ->save(); } + /** + * Check that it's possible to find previously created rule by attribute. + * + * @magentoDbIsolation enabled + * @magentoAppIsolation enabled + * @magentoDataFixture Magento/SalesRule/_files/rule_custom_product_attribute.php + */ + public function testAddAttributeInConditionFilterPositive() + { + $collection = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( + \Magento\SalesRule\Model\ResourceModel\Rule\Collection::class + ); + $collection->addAttributeInConditionFilter('attribute_for_sales_rule_1'); + $item = $collection->getFirstItem(); + $this->assertEquals('50% Off on some attribute', $item->getName()); + } + + /** + * Check that it's not possible to find previously created rule by wrong attribute. + * + * @magentoDbIsolation enabled + * @magentoAppIsolation enabled + * @magentoDataFixture Magento/SalesRule/_files/rule_custom_product_attribute.php + */ + public function testAddAttributeInConditionFilterNegative() + { + $collection = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( + \Magento\SalesRule\Model\ResourceModel\Rule\Collection::class + ); + $collection->addAttributeInConditionFilter('attribute_for_sales_rule_2'); + $this->assertEquals(0, $collection->count()); + } + public function tearDown() { // restore default timezone diff --git a/dev/tests/integration/testsuite/Magento/SalesRule/Model/ResourceModel/RuleTest.php b/dev/tests/integration/testsuite/Magento/SalesRule/Model/ResourceModel/RuleTest.php new file mode 100644 index 0000000000000..a7acb32b9179e --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/SalesRule/Model/ResourceModel/RuleTest.php @@ -0,0 +1,27 @@ +create( + \Magento\SalesRule\Model\ResourceModel\Rule::class + ); + $items = $resource->getActiveAttributes(); + + $this->assertEquals([['attribute_code' => 'attribute_for_sales_rule_1']], $items); + } +} diff --git a/dev/tests/integration/testsuite/Magento/SalesRule/_files/rule_custom_product_attribute.php b/dev/tests/integration/testsuite/Magento/SalesRule/_files/rule_custom_product_attribute.php new file mode 100644 index 0000000000000..779192427da1e --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/SalesRule/_files/rule_custom_product_attribute.php @@ -0,0 +1,76 @@ +create(\Magento\Eav\Model\Entity\Type::class) + ->loadByCode('catalog_category') + ->getId(); + +$attributeData = [ + 'attribute_code' => 'attribute_for_sales_rule_1', + 'entity_type_id' => $entityTypeId, + 'backend_type' => 'varchar', + 'is_required' => 1, + 'is_user_defined' => 1, + 'is_unique' => 0, + 'is_used_for_promo_rules' => 1, +]; + +/** @var \Magento\Eav\Model\Entity\Attribute $attribute */ +$attribute = $objectManager->create(\Magento\Eav\Model\Entity\Attribute::class); +$attribute->setData($attributeData); +$attribute->save(); + +/** @var \Magento\SalesRule\Model\Rule $rule */ +$salesRule = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create(\Magento\SalesRule\Model\Rule::class); +$salesRule->setData( + [ + 'name' => '50% Off on some attribute', + 'is_active' => 1, + 'customer_group_ids' => [\Magento\Customer\Model\GroupManagement::NOT_LOGGED_IN_ID], + 'coupon_type' => \Magento\SalesRule\Model\Rule::COUPON_TYPE_NO_COUPON, + 'simple_action' => 'by_percent', + 'discount_amount' => 50, + 'discount_step' => 0, + 'stop_rules_processing' => 1, + 'website_ids' => [ + \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get( + \Magento\Store\Model\StoreManagerInterface::class + )->getWebsite()->getId() + ] + ] +); + +$salesRule->getConditions()->loadArray([ + 'type' => \Magento\SalesRule\Model\Rule\Condition\Combine::class, + 'attribute' => null, + 'operator' => null, + 'value' => '1', + 'is_value_processed' => null, + 'aggregator' => 'all', + 'conditions' => [ + [ + 'type' => \Magento\SalesRule\Model\Rule\Condition\Product\Found::class, + 'attribute' => null, + 'operator' => null, + 'value' => '0', + 'is_value_processed' => null, + 'aggregator' => 'all', + 'conditions' => [ + [ + 'type' => \Magento\SalesRule\Model\Rule\Condition\Product::class, + 'attribute' => 'attribute_for_sales_rule_1', + 'operator' => '==', + 'value' => '2', + 'is_value_processed' => false, + ], + ], + ], + ], +]); + +$salesRule->save(); From ce6ab5b050669c8c862852211eff8ab60dd12359 Mon Sep 17 00:00:00 2001 From: Mykola Palamar Date: Wed, 25 Jan 2017 18:29:22 +0200 Subject: [PATCH 09/19] MAGETWO-63350: Remove use of unserialize in \Magento\SalesRule\Model\Rule for salesrule field --- .../Magento/SalesRule/Setup/UpgradeData.php | 2 +- .../Unit/Model/ResourceModel/RuleTest.php | 8 +-- .../Model/ResourceModel/RuleTest.php | 54 +++++++++---------- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/app/code/Magento/SalesRule/Setup/UpgradeData.php b/app/code/Magento/SalesRule/Setup/UpgradeData.php index 52cf832beb772..c9bf2a34ae127 100644 --- a/app/code/Magento/SalesRule/Setup/UpgradeData.php +++ b/app/code/Magento/SalesRule/Setup/UpgradeData.php @@ -78,4 +78,4 @@ public function convertSerializedDataToJson($setup) 'actions_serialized' ); } -} \ No newline at end of file +} diff --git a/app/code/Magento/SalesRule/Test/Unit/Model/ResourceModel/RuleTest.php b/app/code/Magento/SalesRule/Test/Unit/Model/ResourceModel/RuleTest.php index 91aaf56a95c1b..0499b1b70a15a 100644 --- a/app/code/Magento/SalesRule/Test/Unit/Model/ResourceModel/RuleTest.php +++ b/app/code/Magento/SalesRule/Test/Unit/Model/ResourceModel/RuleTest.php @@ -208,7 +208,7 @@ public function dataProviderForProductAttributes() return [ [ json_encode([ - 'type' => 'Magento\SalesRule\Model\Rule\Condition\Product', + 'type' => \Magento\SalesRule\Model\Rule\Condition\Product::class, 'attribute' => 'some_attribute', ]), [ @@ -218,11 +218,11 @@ public function dataProviderForProductAttributes() [ json_encode([ [ - 'type' => 'Magento\SalesRule\Model\Rule\Condition\Product', + 'type' => \Magento\SalesRule\Model\Rule\Condition\Product::class, 'attribute' => 'some_attribute', ], [ - 'type' => 'Magento\SalesRule\Model\Rule\Condition\Product', + 'type' => \Magento\SalesRule\Model\Rule\Condition\Product::class, 'attribute' => 'some_attribute2', ], ]), @@ -233,7 +233,7 @@ public function dataProviderForProductAttributes() ], [ json_encode([ - 'type' => 'Magento\SalesRule\Model\Rule\Condition\Product\Found', + 'type' => \Magento\SalesRule\Model\Rule\Condition\Product\Found::class, 'attribute' => 'some_attribute', ]), [] diff --git a/dev/tests/integration/testsuite/Magento/SalesRule/Model/ResourceModel/RuleTest.php b/dev/tests/integration/testsuite/Magento/SalesRule/Model/ResourceModel/RuleTest.php index a7acb32b9179e..3c389be70baba 100644 --- a/dev/tests/integration/testsuite/Magento/SalesRule/Model/ResourceModel/RuleTest.php +++ b/dev/tests/integration/testsuite/Magento/SalesRule/Model/ResourceModel/RuleTest.php @@ -1,27 +1,27 @@ -create( - \Magento\SalesRule\Model\ResourceModel\Rule::class - ); - $items = $resource->getActiveAttributes(); - - $this->assertEquals([['attribute_code' => 'attribute_for_sales_rule_1']], $items); - } -} +create( + \Magento\SalesRule\Model\ResourceModel\Rule::class + ); + $items = $resource->getActiveAttributes(); + + $this->assertEquals([['attribute_code' => 'attribute_for_sales_rule_1']], $items); + } +} From b172c715c74b62bdf4a7bf11421bea3c39ea4c6c Mon Sep 17 00:00:00 2001 From: Mykola Palamar Date: Wed, 25 Jan 2017 19:01:26 +0200 Subject: [PATCH 10/19] MAGETWO-63350: Remove use of unserialize in \Magento\SalesRule\Model\Rule for salesrule field --- app/code/Magento/SalesRule/Setup/UpgradeData.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/SalesRule/Setup/UpgradeData.php b/app/code/Magento/SalesRule/Setup/UpgradeData.php index c9bf2a34ae127..6b395285350cc 100644 --- a/app/code/Magento/SalesRule/Setup/UpgradeData.php +++ b/app/code/Magento/SalesRule/Setup/UpgradeData.php @@ -46,7 +46,7 @@ public function upgrade(ModuleDataSetupInterface $setup, ModuleContextInterface { $setup->startSetup(); - if (version_compare($context->getVersion(), '2.0.1', '<')) { + if (version_compare($context->getVersion(), '2.0.2', '<')) { $this->convertSerializedDataToJson($setup); } From d84df1ed721dbdd0d16504a206c5579e2482d685 Mon Sep 17 00:00:00 2001 From: Roman Ganin Date: Thu, 26 Jan 2017 11:08:54 +0200 Subject: [PATCH 11/19] MAGETWO-63479: Remove use of unserialize in Abstract Rule - coupling fixes --- app/code/Magento/Rule/Model/AbstractModel.php | 1 + app/code/Magento/Rule/Test/Unit/Model/AbstractModelTest.php | 2 ++ 2 files changed, 3 insertions(+) diff --git a/app/code/Magento/Rule/Model/AbstractModel.php b/app/code/Magento/Rule/Model/AbstractModel.php index 618f3b08ea4dc..0c8d7517e4aed 100644 --- a/app/code/Magento/Rule/Model/AbstractModel.php +++ b/app/code/Magento/Rule/Model/AbstractModel.php @@ -95,6 +95,7 @@ abstract public function getActionsInstance(); * @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, diff --git a/app/code/Magento/Rule/Test/Unit/Model/AbstractModelTest.php b/app/code/Magento/Rule/Test/Unit/Model/AbstractModelTest.php index 81e63d65beeb7..652b92fd2491a 100644 --- a/app/code/Magento/Rule/Test/Unit/Model/AbstractModelTest.php +++ b/app/code/Magento/Rule/Test/Unit/Model/AbstractModelTest.php @@ -10,6 +10,8 @@ * Class AbstractModelTest. Unit test for \Magento\Rule\Model\AbstractModel * * @package Magento\Rule\Test\Unit\Model + * + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class AbstractModelTest extends \PHPUnit_Framework_TestCase { From b410f3320ba5e71f7a67b8f85b9921f0cf406865 Mon Sep 17 00:00:00 2001 From: Stanislav Lopukhov Date: Thu, 26 Jan 2017 11:11:47 +0200 Subject: [PATCH 12/19] MAGETWO-63344: Remove use of serialize in \Magento\CatalogRule\Model\Rule for catalogrule table --- .../Magento/CatalogRule/Model/ResourceModel/Rule/Collection.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/code/Magento/CatalogRule/Model/ResourceModel/Rule/Collection.php b/app/code/Magento/CatalogRule/Model/ResourceModel/Rule/Collection.php index d483420b90bd7..f6d846d5e3b5d 100644 --- a/app/code/Magento/CatalogRule/Model/ResourceModel/Rule/Collection.php +++ b/app/code/Magento/CatalogRule/Model/ResourceModel/Rule/Collection.php @@ -107,7 +107,6 @@ protected function mapAssociatedEntities($entityType, $objectField) /** * @return $this - * @throws \Exception */ protected function _afterLoad() { From a9e1a0e77d6944371f775ef5e2389c76576a5515 Mon Sep 17 00:00:00 2001 From: Mykola Palamar Date: Thu, 26 Jan 2017 11:16:19 +0200 Subject: [PATCH 13/19] MAGETWO-63350: Remove use of unserialize in \Magento\SalesRule\Model\Rule for salesrule field --- app/code/Magento/SalesRule/Model/ResourceModel/Rule.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php b/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php index 414891be018bf..e86cf8be6713c 100644 --- a/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php +++ b/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php @@ -73,7 +73,6 @@ public function __construct( \Magento\SalesRule\Model\ResourceModel\Rule\AssociatedEntityMap::class ); $this->_associatedEntitiesMap = $associatedEntitiesMapInstance->getData(); - /* we already have serialized field in AbstractResource, so we don't have to declare it here */ $this->serializer = $serializer ?: \Magento\Framework\App\ObjectManager::getInstance()->get(Json::class); parent::__construct($context, $connectionName); } From 33fa6d015762d958db4f5423efc825a968a16381 Mon Sep 17 00:00:00 2001 From: Stanislav Lopukhov Date: Thu, 26 Jan 2017 13:16:38 +0200 Subject: [PATCH 14/19] MAGETWO-63634: Update Field Data Converter --- lib/internal/Magento/Framework/DB/FieldDataConverter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/DB/FieldDataConverter.php b/lib/internal/Magento/Framework/DB/FieldDataConverter.php index 58e50f725ddd9..4c4c36106f9b2 100644 --- a/lib/internal/Magento/Framework/DB/FieldDataConverter.php +++ b/lib/internal/Magento/Framework/DB/FieldDataConverter.php @@ -69,7 +69,7 @@ public function getSelect( } /** - * Convert field data from one representation to another + * Convert table field data from one representation to another uses DataConverterInterface * * @param AdapterInterface $connection * @param string $table From f366181827d4e6a6745a1aad5b95880e0f205905 Mon Sep 17 00:00:00 2001 From: Mykola Palamar Date: Thu, 26 Jan 2017 18:47:49 +0200 Subject: [PATCH 15/19] MAGETWO-63350: Remove use of unserialize in \Magento\SalesRule\Model\Rule for salesrule field --- .../SalesRule/Model/ResourceModel/Rule.php | 22 +++++++++++++++---- .../Model/ResourceModel/Rule/Collection.php | 16 ++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php b/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php index e86cf8be6713c..05b2374e972c0 100644 --- a/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php +++ b/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php @@ -11,6 +11,8 @@ use Magento\Rule\Model\ResourceModel\AbstractResource; use Magento\Framework\EntityManager\EntityManager; use Magento\Framework\Serialize\Serializer\Json; +use Magento\Framework\EntityManager\MetadataPool; +use Magento\SalesRule\Api\Data\RuleInterface; /** * Sales Rule resource model @@ -51,6 +53,11 @@ class Rule extends AbstractResource */ protected $entityManager; + /** + * @var MetadataPool + */ + private $metadataPool; + /** * @param \Magento\Framework\Model\ResourceModel\Db\Context $context * @param \Magento\Framework\Stdlib\StringUtils $string @@ -58,6 +65,7 @@ class Rule extends AbstractResource * @param string $connectionName * @param \Magento\Framework\DataObject|null $associatedEntityMapInstance * @param Json $serializer Optional parameter for backward compatibility + * @param MetadataPool $metadataPool Optional parameter for backward compatibility */ public function __construct( \Magento\Framework\Model\ResourceModel\Db\Context $context, @@ -65,7 +73,8 @@ public function __construct( \Magento\SalesRule\Model\ResourceModel\Coupon $resourceCoupon, $connectionName = null, \Magento\Framework\DataObject $associatedEntityMapInstance = null, - Json $serializer = null + Json $serializer = null, + MetadataPool $metadataPool = null ) { $this->string = $string; $this->_resourceCoupon = $resourceCoupon; @@ -73,7 +82,8 @@ public function __construct( \Magento\SalesRule\Model\ResourceModel\Rule\AssociatedEntityMap::class ); $this->_associatedEntitiesMap = $associatedEntitiesMapInstance->getData(); - $this->serializer = $serializer ?: \Magento\Framework\App\ObjectManager::getInstance()->get(Json::class); + $this->serializer = $serializer ?: ObjectManager::getInstance()->get(Json::class); + $this->metadataPool = $metadataPool ?: ObjectManager::getInstance()->get(MetadataPool::class); parent::__construct($context, $connectionName); } @@ -305,7 +315,11 @@ public function getActiveAttributes() public function setActualProductAttributes($rule, $attributes) { $connection = $this->getConnection(); - $connection->delete($this->getTable('salesrule_product_attribute'), ['rule_id=?' => $rule->getId()]); + $metadata = $this->metadataPool->getMetadata(RuleInterface::class); + $connection->delete( + $this->getTable('salesrule_product_attribute'), + [$metadata->getLinkField() . '=?' => $rule->getData($metadata->getLinkField())] + ); //Getting attribute IDs for attribute codes $attributeIds = []; @@ -327,7 +341,7 @@ public function setActualProductAttributes($rule, $attributes) foreach ($rule->getWebsiteIds() as $websiteId) { foreach ($attributeIds as $attribute) { $data[] = [ - 'rule_id' => $rule->getId(), + $metadata->getLinkField() => $rule->getData($metadata->getLinkField()), 'website_id' => $websiteId, 'customer_group_id' => $customerGroupId, 'attribute_id' => $attribute, diff --git a/app/code/Magento/SalesRule/Model/ResourceModel/Rule/Collection.php b/app/code/Magento/SalesRule/Model/ResourceModel/Rule/Collection.php index 4a5d0f6e3e418..9a642e76357f7 100644 --- a/app/code/Magento/SalesRule/Model/ResourceModel/Rule/Collection.php +++ b/app/code/Magento/SalesRule/Model/ResourceModel/Rule/Collection.php @@ -282,6 +282,22 @@ public function _initSelect() public function addAttributeInConditionFilter($attributeCode) { $match = sprintf('%%%s%%', substr($this->serializer->serialize(['attribute' => $attributeCode]), 1, -1)); + /** + * Information about conditions and actions stored in table as JSON encoded array + * in fields conditions_serialized and actions_serialized. + * If you want to find rules that contains some particular attribute, the easiest way to do so is serialize + * attribute code in the same way as it stored in the serialized columns and execute SQL search + * with like condition. + * Table + * +-------------------------------------------------------------------+ + * | conditions_serialized | actions_serialized | + * +-------------------------------------------------------------------+ + * | {..."attribute":"attr_name"...} | {..."attribute":"attr_name"...} | + * +---------------------------------|---------------------------------+ + * From attribute code "attr_code", will be generated such SQL: + * `condition_serialized` LIKE '%"attribute":"attr_name"%' + * OR `actions_serialized` LIKE '%"attribute":"attr_name"%' + */ $field = $this->_getMappedField('conditions_serialized'); $cCond = $this->_getConditionSql($field, ['like' => $match]); $field = $this->_getMappedField('actions_serialized'); From 08560d4d13a1397df51d316d8035805278176f73 Mon Sep 17 00:00:00 2001 From: Mykola Palamar Date: Fri, 27 Jan 2017 12:26:36 +0200 Subject: [PATCH 16/19] MAGETWO-63350: Remove use of unserialize in \Magento\SalesRule\Model\Rule for salesrule field --- .../SalesRule/Model/ResourceModel/Rule.php | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php b/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php index 05b2374e972c0..58f2de3a4abd6 100644 --- a/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php +++ b/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php @@ -6,7 +6,6 @@ namespace Magento\SalesRule\Model\ResourceModel; use Magento\Framework\App\ObjectManager; -use \Magento\SalesRule\Model\Rule as SalesRule; use Magento\Framework\Model\AbstractModel; use Magento\Rule\Model\ResourceModel\AbstractResource; use Magento\Framework\EntityManager\EntityManager; @@ -19,13 +18,6 @@ */ class Rule extends AbstractResource { - /** - * Store associated with rule entities information map - * - * @var array - */ - protected $_associatedEntitiesMap = []; - /** * @var array */ @@ -78,10 +70,7 @@ public function __construct( ) { $this->string = $string; $this->_resourceCoupon = $resourceCoupon; - $associatedEntitiesMapInstance = $associatedEntityMapInstance ?: ObjectManager::getInstance()->get( - \Magento\SalesRule\Model\ResourceModel\Rule\AssociatedEntityMap::class - ); - $this->_associatedEntitiesMap = $associatedEntitiesMapInstance->getData(); + $this->_associatedEntitiesMap = $associatedEntityMapInstance ? $associatedEntityMapInstance->getData() : [] ; $this->serializer = $serializer ?: ObjectManager::getInstance()->get(Json::class); $this->metadataPool = $metadataPool ?: ObjectManager::getInstance()->get(MetadataPool::class); parent::__construct($context, $connectionName); @@ -143,7 +132,7 @@ public function _beforeSave(AbstractModel $object) /** * Load an object * - * @param SalesRule|AbstractModel $object + * @param AbstractModel $object * @param mixed $value * @param string $field field to load by (defaults to model id) * @return $this @@ -376,7 +365,6 @@ public function getProductAttributes($serializedString) /** * @param \Magento\Framework\Model\AbstractModel $object * @return $this - * @throws \Exception */ public function save(\Magento\Framework\Model\AbstractModel $object) { @@ -389,7 +377,6 @@ public function save(\Magento\Framework\Model\AbstractModel $object) * * @param \Magento\Framework\Model\AbstractModel $object * @return $this - * @throws \Exception */ public function delete(AbstractModel $object) { From b59364de66023627ddf4af8ebf88c4223edcfe54 Mon Sep 17 00:00:00 2001 From: Mykola Palamar Date: Fri, 27 Jan 2017 13:26:46 +0200 Subject: [PATCH 17/19] MAGETWO-63350: Remove use of unserialize in \Magento\SalesRule\Model\Rule for salesrule field --- .../Magento/SalesRule/Model/ResourceModel/Rule.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php b/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php index 58f2de3a4abd6..65eec664f9bcd 100644 --- a/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php +++ b/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php @@ -15,9 +15,17 @@ /** * Sales Rule resource model + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class Rule extends AbstractResource { + /** + * Store associated with rule entities information map + * + * @var array + */ + protected $_associatedEntitiesMap = []; + /** * @var array */ @@ -70,7 +78,10 @@ public function __construct( ) { $this->string = $string; $this->_resourceCoupon = $resourceCoupon; - $this->_associatedEntitiesMap = $associatedEntityMapInstance ? $associatedEntityMapInstance->getData() : [] ; + $associatedEntitiesMapInstance = $associatedEntityMapInstance ?: ObjectManager::getInstance()->get( + \Magento\SalesRule\Model\ResourceModel\Rule\AssociatedEntityMap::class + ); + $this->_associatedEntitiesMap = $associatedEntitiesMapInstance->getData(); $this->serializer = $serializer ?: ObjectManager::getInstance()->get(Json::class); $this->metadataPool = $metadataPool ?: ObjectManager::getInstance()->get(MetadataPool::class); parent::__construct($context, $connectionName); From 67c2370c5c94bd90cfe54f7216b26f4354451a7d Mon Sep 17 00:00:00 2001 From: Stanislav Lopukhov Date: Tue, 31 Jan 2017 17:36:58 +0200 Subject: [PATCH 18/19] MAGETWO-58456: Remove uses of unserialize in \Magento\Rule\Model\AbstractModel and its child classes and their usages --- lib/internal/Magento/Framework/DB/FieldDataConverter.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/internal/Magento/Framework/DB/FieldDataConverter.php b/lib/internal/Magento/Framework/DB/FieldDataConverter.php index 4c4c36106f9b2..7fd65aa97992d 100644 --- a/lib/internal/Magento/Framework/DB/FieldDataConverter.php +++ b/lib/internal/Magento/Framework/DB/FieldDataConverter.php @@ -50,6 +50,8 @@ public function __construct( * @param QueryModifierInterface|null $queryModifier * * @return Select + * + * @deprecated The method will be removed in MAGETWO-63944. */ public function getSelect( AdapterInterface $connection, From 9a7c892e242da156b304d6a1a8aab1d69550184e Mon Sep 17 00:00:00 2001 From: Roman Ganin Date: Wed, 1 Feb 2017 12:53:28 +0200 Subject: [PATCH 19/19] MAGETWO-63634: Update Field Data Converter - usage of selectFactory in converter instead of direct connection call --- .../Framework/DB/Adapter/Pdo/Mysql.php | 1 - .../Framework/DB/FieldDataConverter.php | 44 ++++++------------- .../DB/Test/Unit/FieldDataConverterTest.php | 18 ++++++-- 3 files changed, 29 insertions(+), 34 deletions(-) diff --git a/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php b/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php index b4cb60f5a5c74..b0173ab7c8a43 100644 --- a/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php +++ b/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php @@ -1387,7 +1387,6 @@ protected function _removeDuplicateEntry($table, $fields, $ids) */ public function select() { -// return new Select($this); return $this->selectFactory->create($this); } diff --git a/lib/internal/Magento/Framework/DB/FieldDataConverter.php b/lib/internal/Magento/Framework/DB/FieldDataConverter.php index 7fd65aa97992d..ffe397a2a4122 100644 --- a/lib/internal/Magento/Framework/DB/FieldDataConverter.php +++ b/lib/internal/Magento/Framework/DB/FieldDataConverter.php @@ -5,6 +5,7 @@ */ namespace Magento\Framework\DB; +use Magento\Framework\App\ObjectManager; use Magento\Framework\DB\Adapter\AdapterInterface; use Magento\Framework\DB\Query\Generator; use Magento\Framework\DB\DataConverter\DataConverterInterface; @@ -26,68 +27,51 @@ class FieldDataConverter */ private $dataConverter; + /** + * @var SelectFactory + */ + private $selectFactory; + /** * Constructor * * @param Generator $queryGenerator * @param DataConverterInterface $dataConverter + * @param SelectFactory $selectFactory */ public function __construct( Generator $queryGenerator, - DataConverterInterface $dataConverter + DataConverterInterface $dataConverter, + SelectFactory $selectFactory = null ) { $this->queryGenerator = $queryGenerator; $this->dataConverter = $dataConverter; + $this->selectFactory = $selectFactory ?: ObjectManager::getInstance()->get(SelectFactory::class); } /** - * Get converter select + * Convert table field data from one representation to another uses DataConverterInterface * * @param AdapterInterface $connection * @param string $table * @param string $identifier * @param string $field * @param QueryModifierInterface|null $queryModifier - * - * @return Select - * - * @deprecated The method will be removed in MAGETWO-63944. + * @return void */ - public function getSelect( + public function convert( AdapterInterface $connection, $table, $identifier, $field, QueryModifierInterface $queryModifier = null ) { - $select = $connection->select() + $select = $this->selectFactory->create($connection) ->from($table, [$identifier, $field]) ->where($field . ' IS NOT NULL'); if ($queryModifier) { $queryModifier->modify($select); } - - return $select; - } - - /** - * Convert table field data from one representation to another uses DataConverterInterface - * - * @param AdapterInterface $connection - * @param string $table - * @param string $identifier - * @param string $field - * @param QueryModifierInterface|null $queryModifier - * @return void - */ - public function convert( - AdapterInterface $connection, - $table, - $identifier, - $field, - QueryModifierInterface $queryModifier = null - ) { - $select = $this->getSelect($connection, $table, $identifier, $field, $queryModifier); $iterator = $this->queryGenerator->generate($identifier, $select); foreach ($iterator as $selectByRange) { $rows = $connection->fetchAll($selectByRange); diff --git a/lib/internal/Magento/Framework/DB/Test/Unit/FieldDataConverterTest.php b/lib/internal/Magento/Framework/DB/Test/Unit/FieldDataConverterTest.php index bddc9250ee611..8b2f589a0358c 100644 --- a/lib/internal/Magento/Framework/DB/Test/Unit/FieldDataConverterTest.php +++ b/lib/internal/Magento/Framework/DB/Test/Unit/FieldDataConverterTest.php @@ -5,6 +5,7 @@ */ namespace Magento\Framework\DB\Test\Unit; +use Magento\Framework\DB\SelectFactory; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; use Magento\Framework\DB\Query\Generator; use Magento\Framework\DB\Adapter\AdapterInterface; @@ -45,6 +46,11 @@ class FieldDataConverterTest extends \PHPUnit_Framework_TestCase */ private $fieldDataConverter; + /** + * @var \Magento\Framework\DB\SelectFactory|\PHPUnit_Framework_MockObject_MockObject + */ + private $selectFactoryMock; + protected function setUp() { $objectManager = new ObjectManager($this); @@ -53,11 +59,16 @@ protected function setUp() $this->dataConverterMock = $this->getMock(DataConverterInterface::class); $this->selectMock = $this->getMock(Select::class, [], [], '', false); $this->queryModifierMock = $this->getMock(QueryModifierInterface::class); + $this->selectFactoryMock = $this->getMockBuilder(SelectFactory::class) + ->disableOriginalConstructor() + ->setMethods([]) + ->getMock(); $this->fieldDataConverter = $objectManager->getObject( FieldDataConverter::class, [ 'queryGenerator' => $this->queryGeneratorMock, - 'dataConverter' => $this->dataConverterMock + 'dataConverter' => $this->dataConverterMock, + 'selectFactory' => $this->selectFactoryMock ] ); } @@ -81,8 +92,9 @@ public function testConvert($useQueryModifier, $numQueryModifierCalls) ] ]; $convertedValue = 'converted value'; - $this->connectionMock->expects($this->once()) - ->method('select') + $this->selectFactoryMock->expects($this->once()) + ->method('create') + ->with($this->connectionMock) ->willReturn($this->selectMock); $this->selectMock->expects($this->once()) ->method('from')