diff --git a/app/code/Magento/CatalogRule/Model/ResourceModel/Rule/Collection.php b/app/code/Magento/CatalogRule/Model/ResourceModel/Rule/Collection.php index 67feaa71c6b5a..f6d846d5e3b5d 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; @@ -96,7 +107,6 @@ protected function mapAssociatedEntities($entityType, $objectField) /** * @return $this - * @throws \Exception */ protected function _afterLoad() { 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/Setup/UpgradeData.php b/app/code/Magento/CatalogRule/Setup/UpgradeData.php new file mode 100644 index 0000000000000..7c34d5a3f54c6 --- /dev/null +++ b/app/code/Magento/CatalogRule/Setup/UpgradeData.php @@ -0,0 +1,82 @@ +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' + ); + } +} 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/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 @@ */ --> - + 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..0c8d7517e4aed 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 + */ + protected $serializer; + /** * Getter for rule combine conditions instance * @@ -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, @@ -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, @@ -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; } @@ -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); } @@ -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); } 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..652b92fd2491a --- /dev/null +++ b/app/code/Magento/Rule/Test/Unit/Model/AbstractModelTest.php @@ -0,0 +1,209 @@ +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/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..65eec664f9bcd 100644 --- a/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php +++ b/app/code/Magento/SalesRule/Model/ResourceModel/Rule.php @@ -6,13 +6,16 @@ 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; +use Magento\Framework\Serialize\Serializer\Json; +use Magento\Framework\EntityManager\MetadataPool; +use Magento\SalesRule\Api\Data\RuleInterface; /** * Sales Rule resource model + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class Rule extends AbstractResource { @@ -50,19 +53,28 @@ class Rule extends AbstractResource */ protected $entityManager; + /** + * @var MetadataPool + */ + private $metadataPool; + /** * @param \Magento\Framework\Model\ResourceModel\Db\Context $context * @param \Magento\Framework\Stdlib\StringUtils $string * @param \Magento\SalesRule\Model\ResourceModel\Coupon $resourceCoupon * @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, \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, + MetadataPool $metadataPool = null ) { $this->string = $string; $this->_resourceCoupon = $resourceCoupon; @@ -70,6 +82,8 @@ public function __construct( \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); } @@ -129,7 +143,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 @@ -157,8 +171,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); @@ -301,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 = []; @@ -323,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, @@ -345,25 +363,19 @@ 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]); } /** * @param \Magento\Framework\Model\AbstractModel $object * @return $this - * @throws \Exception */ public function save(\Magento\Framework\Model\AbstractModel $object) { @@ -376,7 +388,6 @@ public function save(\Magento\Framework\Model\AbstractModel $object) * * @param \Magento\Framework\Model\AbstractModel $object * @return $this - * @throws \Exception */ public function delete(AbstractModel $object) { diff --git a/app/code/Magento/SalesRule/Model/ResourceModel/Rule/Collection.php b/app/code/Magento/SalesRule/Model/ResourceModel/Rule/Collection.php index b884a39a8bd8a..9a642e76357f7 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,23 @@ 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)); + /** + * 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'); 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 ); } diff --git a/app/code/Magento/SalesRule/Setup/UpgradeData.php b/app/code/Magento/SalesRule/Setup/UpgradeData.php new file mode 100644 index 0000000000000..6b395285350cc --- /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.2', '<')) { + $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' + ); + } +} 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..0499b1b70a15a 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::class, + 'attribute' => 'some_attribute', + ]), + [ + 'some_attribute', + ] + ], + [ + json_encode([ + [ + 'type' => \Magento\SalesRule\Model\Rule\Condition\Product::class, + 'attribute' => 'some_attribute', + ], + [ + 'type' => \Magento\SalesRule\Model\Rule\Condition\Product::class, + 'attribute' => 'some_attribute2', + ], + ]), + [ + 'some_attribute', + 'some_attribute2', + ] + ], + [ + json_encode([ + 'type' => \Magento\SalesRule\Model\Rule\Condition\Product\Found::class, + '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/CatalogRule/Model/ResourceModel/Rule/CollectionTest.php b/dev/tests/integration/testsuite/Magento/CatalogRule/Model/ResourceModel/Rule/CollectionTest.php new file mode 100644 index 0000000000000..156f206408dc8 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/CatalogRule/Model/ResourceModel/Rule/CollectionTest.php @@ -0,0 +1,83 @@ +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()); + } +} 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..3c389be70baba --- /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(); 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 2e6e3c20274c6..ffe397a2a4122 100644 --- a/lib/internal/Magento/Framework/DB/FieldDataConverter.php +++ b/lib/internal/Magento/Framework/DB/FieldDataConverter.php @@ -5,10 +5,12 @@ */ 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; use Magento\Framework\DB\Select\QueryModifierInterface; +use Magento\Framework\DB\Select; /** * Convert field data from one representation to another @@ -25,22 +27,30 @@ 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); } /** - * 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 @@ -56,7 +66,7 @@ public function convert( $field, QueryModifierInterface $queryModifier = null ) { - $select = $connection->select() + $select = $this->selectFactory->create($connection) ->from($table, [$identifier, $field]) ->where($field . ' IS NOT NULL'); if ($queryModifier) { 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')