diff --git a/app/code/Magento/Catalog/Controller/Adminhtml/Product/Set/Save.php b/app/code/Magento/Catalog/Controller/Adminhtml/Product/Set/Save.php index c984840e2067daa0f43f06319e9c3a68b1287012..6e432447263ee9d4d6240a6c104b6c599db1e81c 100644 --- a/app/code/Magento/Catalog/Controller/Adminhtml/Product/Set/Save.php +++ b/app/code/Magento/Catalog/Controller/Adminhtml/Product/Set/Save.php @@ -101,6 +101,9 @@ class Save extends \Magento\Catalog\Controller\Adminhtml\Product\Set } $model->save(); $this->messageManager->addSuccess(__('You saved the attribute set.')); + } catch (\Magento\Framework\Exception\AlreadyExistsException $e) { + $this->messageManager->addErrorMessage($e->getMessage()); + $hasError = true; } catch (\Magento\Framework\Exception\LocalizedException $e) { $this->messageManager->addError($e->getMessage()); $hasError = true; diff --git a/app/code/Magento/Eav/Model/Entity/AbstractEntity.php b/app/code/Magento/Eav/Model/Entity/AbstractEntity.php index 878ddf812028825267297aac958c22c14070f7b9..5a88c601e17e3c033040b082f8dfe47b83fd6a03 100644 --- a/app/code/Magento/Eav/Model/Entity/AbstractEntity.php +++ b/app/code/Magento/Eav/Model/Entity/AbstractEntity.php @@ -13,7 +13,8 @@ use Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend; use Magento\Eav\Model\Entity\Attribute\Frontend\AbstractFrontend; use Magento\Eav\Model\Entity\Attribute\Source\AbstractSource; use Magento\Framework\App\Config\Element; -use Magento\Framework\App\ResourceConnection\Config; +use Magento\Framework\DB\Adapter\DuplicateException; +use Magento\Framework\Exception\AlreadyExistsException; use Magento\Framework\Exception\LocalizedException; use Magento\Framework\Model\AbstractModel; use Magento\Framework\Model\ResourceModel\Db\ObjectRelationProcessor; @@ -1108,6 +1109,7 @@ abstract class AbstractEntity extends AbstractResource implements EntityInterfac * @param \Magento\Framework\Model\AbstractModel $object * @return $this * @throws \Exception + * @throws AlreadyExistsException */ public function save(\Magento\Framework\Model\AbstractModel $object) { @@ -1147,6 +1149,10 @@ abstract class AbstractEntity extends AbstractResource implements EntityInterfac } $this->addCommitCallback([$object, 'afterCommitCallback'])->commit(); $object->setHasDataChanges(false); + } catch (DuplicateException $e) { + $this->rollBack(); + $object->setHasDataChanges(true); + throw new AlreadyExistsException(__('Unique constraint violation found'), $e); } catch (\Exception $e) { $this->rollBack(); $object->setHasDataChanges(true); diff --git a/app/code/Magento/Eav/Model/Entity/Attribute/AttributeGroupAlreadyExistsException.php b/app/code/Magento/Eav/Model/Entity/Attribute/AttributeGroupAlreadyExistsException.php new file mode 100644 index 0000000000000000000000000000000000000000..ca80ca089a2ea4cc3b297b4dc933b53049ed31f1 --- /dev/null +++ b/app/code/Magento/Eav/Model/Entity/Attribute/AttributeGroupAlreadyExistsException.php @@ -0,0 +1,15 @@ +<?php +/** + * Copyright © 2016 Magento. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\Eav\Model\Entity\Attribute; + +use Magento\Framework\Exception\AlreadyExistsException; + +/** + * Class AttributeGroupAlreadyExistsException + */ +class AttributeGroupAlreadyExistsException extends AlreadyExistsException +{ +} diff --git a/app/code/Magento/Eav/Model/ResourceModel/Entity/Attribute/Group.php b/app/code/Magento/Eav/Model/ResourceModel/Entity/Attribute/Group.php index 796f4d19f2eb8d90c4625542a33257e3a065a5af..9515bbc66437859d984f126b4d49f483c72f8194 100644 --- a/app/code/Magento/Eav/Model/ResourceModel/Entity/Attribute/Group.php +++ b/app/code/Magento/Eav/Model/ResourceModel/Entity/Attribute/Group.php @@ -5,6 +5,10 @@ */ namespace Magento\Eav\Model\ResourceModel\Entity\Attribute; +use Magento\Eav\Model\Entity\Attribute\AttributeGroupAlreadyExistsException; +use Magento\Framework\DB\Adapter\DuplicateException; +use Magento\Framework\Model\AbstractModel; + /** * Eav Resource Entity Attribute Group * @@ -50,10 +54,10 @@ class Group extends \Magento\Framework\Model\ResourceModel\Db\AbstractDb /** * Perform actions before object save * - * @param \Magento\Framework\Model\AbstractModel $object + * @param AbstractModel $object * @return \Magento\Framework\Model\ResourceModel\Db\AbstractDb */ - protected function _beforeSave(\Magento\Framework\Model\AbstractModel $object) + protected function _beforeSave(AbstractModel $object) { if (!$object->getSortOrder()) { $object->setSortOrder($this->_getMaxSortOrder($object) + 1); @@ -64,10 +68,10 @@ class Group extends \Magento\Framework\Model\ResourceModel\Db\AbstractDb /** * Perform actions after object save * - * @param \Magento\Framework\Model\AbstractModel $object + * @param AbstractModel $object * @return \Magento\Framework\Model\ResourceModel\Db\AbstractDb */ - protected function _afterSave(\Magento\Framework\Model\AbstractModel $object) + protected function _afterSave(AbstractModel $object) { if ($object->getAttributes()) { foreach ($object->getAttributes() as $attribute) { @@ -82,7 +86,7 @@ class Group extends \Magento\Framework\Model\ResourceModel\Db\AbstractDb /** * Retrieve max sort order * - * @param \Magento\Framework\Model\AbstractModel $object + * @param AbstractModel $object * @return int */ protected function _getMaxSortOrder($object) @@ -130,4 +134,38 @@ class Group extends \Magento\Framework\Model\ResourceModel\Db\AbstractDb return $this; } + + /** + * {@inheritdoc} + */ + protected function saveNewObject(AbstractModel $object) + { + try { + return parent::saveNewObject($object); + } catch (DuplicateException $e) { + throw new AttributeGroupAlreadyExistsException( + __( + 'Attribute group with same code already exist. Please rename "%1" group', + $object->getAttributeGroupName() + ) + ); + } + } + + /** + * {@inheritdoc} + */ + protected function updateObject(AbstractModel $object) + { + try { + return parent::updateObject($object); + } catch (DuplicateException $e) { + throw new AttributeGroupAlreadyExistsException( + __( + 'Attribute group with same code already exist. Please rename "%1" group', + $object->getAttributeGroupName() + ) + ); + } + } } diff --git a/app/code/Magento/Eav/Setup/UpgradeSchema.php b/app/code/Magento/Eav/Setup/UpgradeSchema.php new file mode 100644 index 0000000000000000000000000000000000000000..2b52bba7c2efbd379851ddf16a0cafbc077dcaf2 --- /dev/null +++ b/app/code/Magento/Eav/Setup/UpgradeSchema.php @@ -0,0 +1,48 @@ +<?php +/** + * Copyright © 2016 Magento. All rights reserved. + * See COPYING.txt for license details. + */ + +namespace Magento\Eav\Setup; + +use Magento\Framework\Setup\UpgradeSchemaInterface; +use Magento\Framework\Setup\ModuleContextInterface; +use Magento\Framework\Setup\SchemaSetupInterface; + +/** + * Upgrade the Eav module DB scheme + */ +class UpgradeSchema implements UpgradeSchemaInterface +{ + /** + * {@inheritdoc} + */ + public function upgrade(SchemaSetupInterface $setup, ModuleContextInterface $context) + { + $setup->startSetup(); + + if (version_compare($context->getVersion(), '2.1.0', '<')) { + $this->addUniqueKeyToEavAttributeGroupTable($setup); + } + $setup->endSetup(); + } + + /** + * @param SchemaSetupInterface $setup + * @return void + */ + private function addUniqueKeyToEavAttributeGroupTable(SchemaSetupInterface $setup) + { + $setup->getConnection()->addIndex( + $setup->getTable('eav_attribute_group'), + $setup->getIdxName( + 'catalog_category_product', + ['attribute_set_id', 'attribute_group_code'], + \Magento\Framework\DB\Adapter\AdapterInterface::INDEX_TYPE_UNIQUE + ), + ['attribute_set_id', 'attribute_group_code'], + \Magento\Framework\DB\Adapter\AdapterInterface::INDEX_TYPE_UNIQUE + ); + } +} diff --git a/app/code/Magento/Eav/Test/Unit/Model/Entity/AbstractEntityTest.php b/app/code/Magento/Eav/Test/Unit/Model/Entity/AbstractEntityTest.php index f39eeb892757645df9fc3157894249e18d8c4803..7628207e25a52c2ea7cd595bd47f65fb0d18ef81 100644 --- a/app/code/Magento/Eav/Test/Unit/Model/Entity/AbstractEntityTest.php +++ b/app/code/Magento/Eav/Test/Unit/Model/Entity/AbstractEntityTest.php @@ -5,13 +5,17 @@ */ namespace Magento\Eav\Test\Unit\Model\Entity; +use Magento\Eav\Model\Entity\AbstractEntity; +use Magento\Framework\DB\Adapter\AdapterInterface; +use Magento\Framework\DB\Adapter\DuplicateException; +use Magento\Framework\Model\AbstractModel; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; class AbstractEntityTest extends \PHPUnit_Framework_TestCase { /** * Entity model to be tested - * @var \Magento\Eav\Model\Entity\AbstractEntity|\PHPUnit_Framework_MockObject_MockObject + * @var AbstractEntity|\PHPUnit_Framework_MockObject_MockObject */ protected $_model; @@ -23,11 +27,11 @@ class AbstractEntityTest extends \PHPUnit_Framework_TestCase $objectManager = new ObjectManager($this); $this->eavConfig = $this->getMock(\Magento\Eav\Model\Config::class, [], [], '', false); $arguments = $objectManager->getConstructArguments( - \Magento\Eav\Model\Entity\AbstractEntity::class, + AbstractEntity::class, ['eavConfig' => $this->eavConfig] ); $this->_model = $this->getMockForAbstractClass( - \Magento\Eav\Model\Entity\AbstractEntity::class, + AbstractEntity::class, $arguments ); } @@ -113,7 +117,7 @@ class AbstractEntityTest extends \PHPUnit_Framework_TestCase /** * Get adapter mock * - * @return \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\DB\Adapter\AdapterInterface + * @return \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\DB\Adapter\Pdo\Mysql */ protected function _getConnectionMock() { @@ -300,7 +304,7 @@ class AbstractEntityTest extends \PHPUnit_Framework_TestCase $objectManager = new ObjectManager($this); $this->eavConfig = $this->getMock(\Magento\Eav\Model\Config::class, [], [], '', false); $arguments = $objectManager->getConstructArguments( - \Magento\Eav\Model\Entity\AbstractEntity::class, + AbstractEntity::class, [ 'eavConfig' => $eavConfig, 'data' => [ @@ -310,8 +314,8 @@ class AbstractEntityTest extends \PHPUnit_Framework_TestCase ] ] ); - /** @var $model \Magento\Framework\Model\AbstractModel|\PHPUnit_Framework_MockObject_MockObject */ - $model = $this->getMockBuilder(\Magento\Eav\Model\Entity\AbstractEntity::class) + /** @var $model AbstractEntity|\PHPUnit_Framework_MockObject_MockObject */ + $model = $this->getMockBuilder(AbstractEntity::class) ->setConstructorArgs($arguments) ->setMethods(['_getValue', 'beginTransaction', 'commit', 'rollback', 'getConnection']) ->getMock(); @@ -353,4 +357,30 @@ class AbstractEntityTest extends \PHPUnit_Framework_TestCase ] ]; } + + /** + * @expectedException \Magento\Framework\Exception\AlreadyExistsException + */ + public function testDuplicateExceptionProcessingOnSave() + { + $connection = $this->getMock(AdapterInterface::class); + $connection->expects($this->once())->method('rollback'); + + /** @var AbstractEntity|\PHPUnit_Framework_MockObject_MockObject $model */ + $model = $this->getMockBuilder(AbstractEntity::class) + ->disableOriginalConstructor() + ->setMethods(['getConnection']) + ->getMockForAbstractClass(); + $model->expects($this->any())->method('getConnection')->willReturn($connection); + + /** @var AbstractModel|\PHPUnit_Framework_MockObject_MockObject $object */ + $object = $this->getMockBuilder(AbstractModel::class) + ->disableOriginalConstructor() + ->getMock(); + $object->expects($this->once())->method('hasDataChanges')->willReturn(true); + $object->expects($this->once())->method('beforeSave')->willThrowException(new DuplicateException()); + $object->expects($this->once())->method('setHasDataChanges')->with(true); + + $model->save($object); + } } diff --git a/app/code/Magento/Eav/etc/module.xml b/app/code/Magento/Eav/etc/module.xml index c1c313d91501989578f03b404ae91acc2d0ccbb1..d03606d1eeb900a1dfec428c172cba0b47a01fc0 100644 --- a/app/code/Magento/Eav/etc/module.xml +++ b/app/code/Magento/Eav/etc/module.xml @@ -6,7 +6,7 @@ */ --> <config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd"> - <module name="Magento_Eav" setup_version="2.0.0"> + <module name="Magento_Eav" setup_version="2.1.0"> <sequence> <module name="Magento_Store"/> </sequence> diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/Product/Set/SaveTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/Product/Set/SaveTest.php new file mode 100644 index 0000000000000000000000000000000000000000..4738e1886be3e6d9ab64f65cd7f241b63453a741 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/Product/Set/SaveTest.php @@ -0,0 +1,64 @@ +<?php +/** + * Copyright © 2016 Magento. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\Catalog\Controller\Adminhtml\Product\Set; + +use Magento\Eav\Api\AttributeSetRepositoryInterface; +use Magento\Eav\Api\Data\AttributeSetInterface; +use Magento\Framework\Api\SearchCriteriaBuilder; +use Magento\TestFramework\Helper\Bootstrap; + +class SaveTest extends \Magento\TestFramework\TestCase\AbstractBackendController +{ + /** + * @magentoDataFixture Magento/Catalog/_files/attribute_set_with_renamed_group.php + */ + public function testAlreadyExistsExceptionProcessingWhenGroupCodeIsDuplicated() + { + $attributeSet = $this->getAttributeSetByName('attribute_set_test'); + $this->assertNotEmpty($attributeSet, 'Attribute set with name "attribute_set_test" is missed'); + + $this->getRequest()->setPostValue('data', json_encode([ + 'attribute_set_name' => 'attribute_set_test', + 'groups' => [ + ['ynode-418', 'attribute-group-name', 1], + ], + 'attributes' => [ + ['9999', 'ynode-418', 1, null] + ], + 'not_attributes' => [], + 'removeGroups' => [], + ])); + $this->dispatch('backend/catalog/product_set/save/id/' . $attributeSet->getAttributeSetId()); + + $jsonResponse = json_decode($this->getResponse()->getBody()); + $this->assertNotNull($jsonResponse); + $this->assertEquals(1, $jsonResponse->error); + $this->assertContains( + 'Attribute group with same code already exist. Please rename "attribute-group-name" group', + $jsonResponse->message + ); + } + + /** + * @param string $attributeSetName + * @return AttributeSetInterface|null + */ + protected function getAttributeSetByName($attributeSetName) + { + $objectManager = Bootstrap::getObjectManager(); + + /** @var SearchCriteriaBuilder $searchCriteriaBuilder */ + $searchCriteriaBuilder = $objectManager->get(SearchCriteriaBuilder::class); + $searchCriteriaBuilder->addFilter('attribute_set_name', $attributeSetName); + + /** @var AttributeSetRepositoryInterface $attributeSetRepository */ + $attributeSetRepository = $objectManager->get(AttributeSetRepositoryInterface::class); + $result = $attributeSetRepository->getList($searchCriteriaBuilder->create()); + + $items = $result->getItems(); + return $result->getTotalCount() ? array_pop($items) : null; + } +} diff --git a/dev/tests/integration/testsuite/Magento/Catalog/_files/attribute_set_with_renamed_group.php b/dev/tests/integration/testsuite/Magento/Catalog/_files/attribute_set_with_renamed_group.php new file mode 100644 index 0000000000000000000000000000000000000000..d73baa9e70e023d55e0a3306a03f8b7a56666b86 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Catalog/_files/attribute_set_with_renamed_group.php @@ -0,0 +1,54 @@ +<?php +/** + * Copyright © 2016 Magento. All rights reserved. + * See COPYING.txt for license details. + */ +use Magento\Catalog\Api\AttributeSetRepositoryInterface; +use Magento\Eav\Api\AttributeGroupRepositoryInterface; +use Magento\Eav\Api\Data\AttributeGroupInterface; +use Magento\Eav\Api\Data\AttributeGroupInterfaceFactory; +use Magento\Eav\Api\Data\AttributeSetInterface; +use Magento\Eav\Api\Data\AttributeSetInterfaceFactory; +use Magento\Eav\Model\Entity\Type; +use Magento\Framework\Api\DataObjectHelper; +use Magento\TestFramework\Helper\Bootstrap; + +$objectManager = Bootstrap::getObjectManager(); +$attributeSetFactory = $objectManager->get(AttributeSetInterfaceFactory::class); +$attributeGroupFactory = $objectManager->get(AttributeGroupInterfaceFactory::class); +/** @var DataObjectHelper $dataObjectHelper */ +$dataObjectHelper = $objectManager->get(DataObjectHelper::class); +/** @var AttributeGroupRepositoryInterface $attributeGroupRepository */ +$attributeGroupRepository = $objectManager->get(AttributeGroupRepositoryInterface::class); +/** @var AttributeSetRepositoryInterface $attributeSetRepository */ +$attributeSetRepository = $objectManager->get(AttributeSetRepositoryInterface::class); + +/** @var AttributeSetInterface $attributeSet */ +$attributeSet = $attributeSetFactory->create(); +$entityTypeId = $objectManager->create(Type::class)->loadByCode('catalog_product')->getId(); +$dataObjectHelper->populateWithArray( + $attributeSet, + [ + 'attribute_set_name' => 'attribute_set_test', + 'entity_type_id' => $entityTypeId, + ], + AttributeSetInterface::class +); +$attributeSetRepository->save($attributeSet); + +/** @var AttributeGroupInterface $attributeGroup */ +$attributeGroup = $attributeGroupFactory->create(); +$dataObjectHelper->populateWithArray( + $attributeGroup, + [ + 'attribute_set_id' => $attributeSet->getAttributeSetId(), + 'attribute_group_name' => 'attribute-group-name', + 'default_id' => 1, + ], + AttributeGroupInterface::class +); +$attributeGroupRepository->save($attributeGroup); + +// during renaming group code is not changed +$attributeGroup->setAttributeGroupName('attribute-group-renamed'); +$attributeGroupRepository->save($attributeGroup); diff --git a/dev/tests/static/testsuite/Magento/Test/Integrity/_files/blacklist/exception_hierarchy.txt b/dev/tests/static/testsuite/Magento/Test/Integrity/_files/blacklist/exception_hierarchy.txt index 9ae1f7fc1452d3156aa7308ee3f81d194e99bbce..a50845d3884adf110ff8c36831e6ef8ac63670ca 100644 --- a/dev/tests/static/testsuite/Magento/Test/Integrity/_files/blacklist/exception_hierarchy.txt +++ b/dev/tests/static/testsuite/Magento/Test/Integrity/_files/blacklist/exception_hierarchy.txt @@ -6,3 +6,4 @@ \Magento\Framework\DB\Adapter\ConnectionException \Magento\Framework\DB\Adapter\DeadlockException \Magento\Framework\DB\Adapter\LockWaitException +\Magento\Framework\DB\Adapter\DuplicateException diff --git a/dev/tests/static/testsuite/Magento/Test/Legacy/_files/security/blacklist.php b/dev/tests/static/testsuite/Magento/Test/Legacy/_files/security/blacklist.php index f055f0a732c664dd8ed109d7ec52b178c537a4d0..d120a4543b9ddced597d330ab489de15c80cdb86 100644 --- a/dev/tests/static/testsuite/Magento/Test/Legacy/_files/security/blacklist.php +++ b/dev/tests/static/testsuite/Magento/Test/Legacy/_files/security/blacklist.php @@ -4,5 +4,6 @@ * See COPYING.txt for license details. */ return [ - '/Test\/Unit/' + '/Test\/Unit/', + '/lib\/internal\/Magento\/Framework\/DB\/Adapter\/Pdo\/Mysql\.php/', ]; diff --git a/lib/internal/Magento/Framework/DB/Adapter/DuplicateException.php b/lib/internal/Magento/Framework/DB/Adapter/DuplicateException.php new file mode 100644 index 0000000000000000000000000000000000000000..06c4dfcc30ab26572b0faae1d90d1939989455ba --- /dev/null +++ b/lib/internal/Magento/Framework/DB/Adapter/DuplicateException.php @@ -0,0 +1,13 @@ +<?php +/** + * Copyright © 2016 Magento. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\Framework\DB\Adapter; + +/** + * Database duplicate exception + */ +class DuplicateException extends \Zend_Db_Adapter_Exception +{ +} diff --git a/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php b/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php index 6399824590a696980b641abcd3d0fc5b4e09cf42..cfbeb4f54c9e29d2f7b320b57c51ae7a7b31a676 100644 --- a/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php +++ b/lib/internal/Magento/Framework/DB/Adapter/Pdo/Mysql.php @@ -14,6 +14,7 @@ use Magento\Framework\Cache\FrontendInterface; use Magento\Framework\DB\Adapter\AdapterInterface; use Magento\Framework\DB\Adapter\ConnectionException; use Magento\Framework\DB\Adapter\DeadlockException; +use Magento\Framework\DB\Adapter\DuplicateException; use Magento\Framework\DB\Adapter\LockWaitException; use Magento\Framework\DB\Ddl\Table; use Magento\Framework\DB\ExpressionConverter; @@ -232,6 +233,8 @@ class Mysql extends \Zend_Db_Adapter_Pdo_Mysql implements AdapterInterface 1205 => LockWaitException::class, // SQLSTATE[40001]: Serialization failure: 1213 Deadlock found when trying to get lock 1213 => DeadlockException::class, + // SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry + 1062 => DuplicateException::class, ]; try { parent::__construct($config); @@ -465,7 +468,7 @@ class Mysql extends \Zend_Db_Adapter_Pdo_Mysql implements AdapterInterface * @param mixed $bind An array of data or data itself to bind to the placeholders. * @return \Zend_Db_Statement_Pdo|void * @throws \Zend_Db_Adapter_Exception To re-throw \PDOException. - * @throws LocalizedException In case multiple queries are attempted at once, to protect from SQL injection + * @throws \Zend_Db_Statement_Exception * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ protected function _query($sql, $bind = []) diff --git a/lib/internal/Magento/Framework/EntityManager/Operation/Create.php b/lib/internal/Magento/Framework/EntityManager/Operation/Create.php index 7e3dfb2e753caa4ebd8efc72c159b0b3a3d521d4..552fe7bca86f58fb722fab1b08294b4ee41d3734 100644 --- a/lib/internal/Magento/Framework/EntityManager/Operation/Create.php +++ b/lib/internal/Magento/Framework/EntityManager/Operation/Create.php @@ -5,7 +5,7 @@ */ namespace Magento\Framework\EntityManager\Operation; -use Magento\Framework\EntityManager\Operation\CreateInterface; +use Magento\Framework\DB\Adapter\DuplicateException; use Magento\Framework\EntityManager\Operation\Create\CreateMain; use Magento\Framework\EntityManager\Operation\Create\CreateAttributes; use Magento\Framework\EntityManager\Operation\Create\CreateExtensions; @@ -13,6 +13,8 @@ use Magento\Framework\EntityManager\MetadataPool; use Magento\Framework\EntityManager\EventManager; use Magento\Framework\EntityManager\TypeResolver; use Magento\Framework\App\ResourceConnection; +use Magento\Framework\Exception\AlreadyExistsException; +use Magento\Framework\Phrase; /** * Class Create @@ -86,6 +88,7 @@ class Create implements CreateInterface * @param array $arguments * @return object * @throws \Exception + * @throws AlreadyExistsException */ public function execute($entity, $arguments = []) { @@ -114,6 +117,9 @@ class Create implements CreateInterface ] ); $connection->commit(); + } catch (DuplicateException $e) { + $connection->rollBack(); + throw new AlreadyExistsException(new Phrase('Unique constraint violation found'), $e); } catch (\Exception $e) { $connection->rollBack(); throw $e; diff --git a/lib/internal/Magento/Framework/EntityManager/Operation/Update.php b/lib/internal/Magento/Framework/EntityManager/Operation/Update.php index 22abb464f8cd9ad96713899ca95fc0a3dc87e782..dbc8dc63873498888a40396795f9d6a0b9edca19 100644 --- a/lib/internal/Magento/Framework/EntityManager/Operation/Update.php +++ b/lib/internal/Magento/Framework/EntityManager/Operation/Update.php @@ -5,7 +5,7 @@ */ namespace Magento\Framework\EntityManager\Operation; -use Magento\Framework\EntityManager\Operation\UpdateInterface; +use Magento\Framework\DB\Adapter\DuplicateException; use Magento\Framework\EntityManager\Operation\Update\UpdateMain; use Magento\Framework\EntityManager\Operation\Update\UpdateAttributes; use Magento\Framework\EntityManager\Operation\Update\UpdateExtensions; @@ -13,6 +13,8 @@ use Magento\Framework\EntityManager\MetadataPool; use Magento\Framework\EntityManager\EventManager; use Magento\Framework\EntityManager\TypeResolver; use Magento\Framework\App\ResourceConnection; +use Magento\Framework\Exception\AlreadyExistsException; +use Magento\Framework\Phrase; /** * Class Update @@ -114,6 +116,9 @@ class Update implements UpdateInterface ] ); $connection->commit(); + } catch (DuplicateException $e) { + $connection->rollBack(); + throw new AlreadyExistsException(new Phrase('Unique constraint violation found'), $e); } catch (\Exception $e) { $connection->rollBack(); throw $e; diff --git a/lib/internal/Magento/Framework/EntityManager/Test/Unit/Operation/CreateTest.php b/lib/internal/Magento/Framework/EntityManager/Test/Unit/Operation/CreateTest.php new file mode 100644 index 0000000000000000000000000000000000000000..e45e8ffac1934859b16fd040505865d2f078eed3 --- /dev/null +++ b/lib/internal/Magento/Framework/EntityManager/Test/Unit/Operation/CreateTest.php @@ -0,0 +1,78 @@ +<?php +/** + * Copyright © 2016 Magento. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\Framework\EntityManager\Test\Unit\Operation; + +use Magento\Framework\App\ResourceConnection; +use Magento\Framework\DataObject; +use Magento\Framework\DB\Adapter\AdapterInterface; +use Magento\Framework\DB\Adapter\DuplicateException; +use Magento\Framework\EntityManager\EntityMetadataInterface; +use Magento\Framework\EntityManager\MetadataPool; +use Magento\Framework\EntityManager\Operation\Create; +use Magento\Framework\EntityManager\Operation\Create\CreateMain; +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; + +class CreateTest extends \PHPUnit_Framework_TestCase +{ + /** + * @var MetadataPool|\PHPUnit_Framework_MockObject_MockObject + */ + private $metadataPool; + + /** + * @var ResourceConnection|\PHPUnit_Framework_MockObject_MockObject + */ + private $resourceConnection; + + /** + * @var CreateMain|\PHPUnit_Framework_MockObject_MockObject + */ + private $createMain; + + /** + * @var Create + */ + private $create; + + public function setUp() + { + $this->metadataPool = $this->getMockBuilder(MetadataPool::class) + ->disableOriginalConstructor() + ->getMock(); + $this->resourceConnection = $this->getMockBuilder(ResourceConnection::class) + ->disableOriginalConstructor() + ->getMock(); + $this->createMain = $this->getMockBuilder(CreateMain::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->create = (new ObjectManager($this))->getObject(Create::class, [ + 'metadataPool' => $this->metadataPool, + 'resourceConnection' => $this->resourceConnection, + 'createMain' => $this->createMain, + ]); + } + + /** + * @expectedException \Magento\Framework\Exception\AlreadyExistsException + */ + public function testDuplicateExceptionProcessingOnExecute() + { + $metadata = $this->getMock(EntityMetadataInterface::class); + $this->metadataPool->expects($this->any())->method('getMetadata')->willReturn($metadata); + + $connection = $this->getMock(AdapterInterface::class); + $connection->expects($this->once())->method('rollback'); + $this->resourceConnection->expects($this->any())->method('getConnectionByName')->willReturn($connection); + + $this->createMain->expects($this->once())->method('execute')->willThrowException(new DuplicateException()); + + $entity = $this->getMockBuilder(DataObject::class) + ->disableOriginalConstructor() + ->getMock(); + $this->create->execute($entity); + } +} diff --git a/lib/internal/Magento/Framework/EntityManager/Test/Unit/Operation/UpdateTest.php b/lib/internal/Magento/Framework/EntityManager/Test/Unit/Operation/UpdateTest.php new file mode 100644 index 0000000000000000000000000000000000000000..2ec78c2560a28b3d0cd2043b8fa09fe653e49be8 --- /dev/null +++ b/lib/internal/Magento/Framework/EntityManager/Test/Unit/Operation/UpdateTest.php @@ -0,0 +1,78 @@ +<?php +/** + * Copyright © 2016 Magento. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\Framework\EntityManager\Test\Unit\Operation; + +use Magento\Framework\App\ResourceConnection; +use Magento\Framework\DataObject; +use Magento\Framework\DB\Adapter\AdapterInterface; +use Magento\Framework\DB\Adapter\DuplicateException; +use Magento\Framework\EntityManager\EntityMetadataInterface; +use Magento\Framework\EntityManager\MetadataPool; +use Magento\Framework\EntityManager\Operation\Update; +use Magento\Framework\EntityManager\Operation\Update\UpdateMain; +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; + +class UpdateTest extends \PHPUnit_Framework_TestCase +{ + /** + * @var MetadataPool|\PHPUnit_Framework_MockObject_MockObject + */ + private $metadataPool; + + /** + * @var ResourceConnection|\PHPUnit_Framework_MockObject_MockObject + */ + private $resourceConnection; + + /** + * @var UpdateMain|\PHPUnit_Framework_MockObject_MockObject + */ + private $updateMain; + + /** + * @var Update + */ + private $update; + + public function setUp() + { + $this->metadataPool = $this->getMockBuilder(MetadataPool::class) + ->disableOriginalConstructor() + ->getMock(); + $this->resourceConnection = $this->getMockBuilder(ResourceConnection::class) + ->disableOriginalConstructor() + ->getMock(); + $this->updateMain = $this->getMockBuilder(UpdateMain::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->update = (new ObjectManager($this))->getObject(Update::class, [ + 'metadataPool' => $this->metadataPool, + 'resourceConnection' => $this->resourceConnection, + 'updateMain' => $this->updateMain, + ]); + } + + /** + * @expectedException \Magento\Framework\Exception\AlreadyExistsException + */ + public function testDuplicateExceptionProcessingOnExecute() + { + $metadata = $this->getMock(EntityMetadataInterface::class); + $this->metadataPool->expects($this->any())->method('getMetadata')->willReturn($metadata); + + $connection = $this->getMock(AdapterInterface::class); + $connection->expects($this->once())->method('rollback'); + $this->resourceConnection->expects($this->any())->method('getConnectionByName')->willReturn($connection); + + $this->updateMain->expects($this->once())->method('execute')->willThrowException(new DuplicateException()); + + $entity = $this->getMockBuilder(DataObject::class) + ->disableOriginalConstructor() + ->getMock(); + $this->update->execute($entity); + } +} diff --git a/lib/internal/Magento/Framework/Exception/AlreadyExistsException.php b/lib/internal/Magento/Framework/Exception/AlreadyExistsException.php index dd39ec8c709a75fa4c4a8721dd8fdbfed4499c2f..18afdacf53de2676525f70fd76e5b94b3671dddb 100644 --- a/lib/internal/Magento/Framework/Exception/AlreadyExistsException.php +++ b/lib/internal/Magento/Framework/Exception/AlreadyExistsException.php @@ -3,9 +3,24 @@ * Copyright © 2016 Magento. All rights reserved. * See COPYING.txt for license details. */ - namespace Magento\Framework\Exception; +use Magento\Framework\Phrase; + +/** + * Class AlreadyExistsException + */ class AlreadyExistsException extends LocalizedException { + /** + * @param Phrase $phrase + * @param \Exception $cause + */ + public function __construct(Phrase $phrase = null, \Exception $cause = null) + { + if ($phrase === null) { + $phrase = new Phrase('Unique constraint violation found'); + } + parent::__construct($phrase, $cause); + } } diff --git a/lib/internal/Magento/Framework/Model/ResourceModel/Db/AbstractDb.php b/lib/internal/Magento/Framework/Model/ResourceModel/Db/AbstractDb.php index 7febcb450f9720d9999f19704808df90ca69deea..4cd88e356e9cad79a6f0209bb2bab685ffe5f73c 100644 --- a/lib/internal/Magento/Framework/Model/ResourceModel/Db/AbstractDb.php +++ b/lib/internal/Magento/Framework/Model/ResourceModel/Db/AbstractDb.php @@ -10,6 +10,8 @@ use Magento\Framework\App\ResourceConnection; use Magento\Framework\Exception\AlreadyExistsException; use Magento\Framework\Exception\LocalizedException; use Magento\Framework\Model\ResourceModel\AbstractResource; +use Magento\Framework\DB\Adapter\DuplicateException; +use Magento\Framework\Phrase; /** * Abstract resource model class @@ -379,6 +381,7 @@ abstract class AbstractDb extends AbstractResource * @return $this * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @throws \Exception + * @throws AlreadyExistsException * @api */ public function save(\Magento\Framework\Model\AbstractModel $object) @@ -413,6 +416,10 @@ abstract class AbstractDb extends AbstractResource } $this->addCommitCallback([$object, 'afterCommitCallback'])->commit(); $object->setHasDataChanges(false); + } catch (DuplicateException $e) { + $this->rollBack(); + $object->setHasDataChanges(true); + throw new AlreadyExistsException(new Phrase('Unique constraint violation found'), $e); } catch (\Exception $e) { $this->rollBack(); $object->setHasDataChanges(true); diff --git a/lib/internal/Magento/Framework/Model/Test/Unit/ResourceModel/Db/AbstractDbTest.php b/lib/internal/Magento/Framework/Model/Test/Unit/ResourceModel/Db/AbstractDbTest.php index 0a2e6f2262173f244dc0dddf8c53fa44c9bf3558..d13bcdc539aa8c82d7af3b24ea1d32ed93e8431b 100644 --- a/lib/internal/Magento/Framework/Model/Test/Unit/ResourceModel/Db/AbstractDbTest.php +++ b/lib/internal/Magento/Framework/Model/Test/Unit/ResourceModel/Db/AbstractDbTest.php @@ -7,6 +7,10 @@ // @codingStandardsIgnoreFile namespace Magento\Framework\Model\Test\Unit\ResourceModel\Db; +use Magento\Framework\DB\Adapter\AdapterInterface; +use Magento\Framework\DB\Adapter\DuplicateException; +use Magento\Framework\Model\AbstractModel; +use Magento\Framework\Model\ResourceModel\Db\AbstractDb; /** * @SuppressWarnings(PHPMD.CouplingBetweenObjects) @@ -14,7 +18,7 @@ namespace Magento\Framework\Model\Test\Unit\ResourceModel\Db; class AbstractDbTest extends \PHPUnit_Framework_TestCase { /** - * @var \Magento\Framework\Model\ResourceModel\Db\AbstractDb + * @var AbstractDb */ protected $_model; @@ -63,7 +67,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase ->willReturn($this->transactionManagerMock); $this->_model = $this->getMockForAbstractClass( - \Magento\Framework\Model\ResourceModel\Db\AbstractDb::class, + AbstractDb::class, [$contextMock], '', true, @@ -116,7 +120,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase public function testAddUniqueFieldArray() { $this->assertInstanceOf( - \Magento\Framework\Model\ResourceModel\Db\AbstractDb::class, + AbstractDb::class, $this->_model->addUniqueField(['someField']) ); } @@ -134,7 +138,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase { $data = 'MainTableName'; $idFieldNameProperty = new \ReflectionProperty( - \Magento\Framework\Model\ResourceModel\Db\AbstractDb::class, '_idFieldName' + AbstractDb::class, '_idFieldName' ); $idFieldNameProperty->setAccessible(true); $idFieldNameProperty->setValue($this->_model, $data); @@ -158,7 +162,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase public function testGetMainTable($tableName, $expectedResult) { $mainTableProperty = new \ReflectionProperty( - \Magento\Framework\Model\ResourceModel\Db\AbstractDb::class, + AbstractDb::class, '_mainTable' ); $mainTableProperty->setAccessible(true); @@ -195,7 +199,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase $this->returnValue('tableName') ); $tablesProperty = new \ReflectionProperty( - \Magento\Framework\Model\ResourceModel\Db\AbstractDb::class, + AbstractDb::class, '_tables' ); $tablesProperty->setAccessible(true); @@ -215,7 +219,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase */ public function testGetChecksum($checksum, $expected) { - $connectionMock = $this->getMock(\Magento\Framework\DB\Adapter\AdapterInterface::class, [], [], '', false); + $connectionMock = $this->getMock(AdapterInterface::class, [], [], '', false); $connectionMock->expects($this->once())->method('getTablesChecksum')->with($checksum)->will( $this->returnValue([$checksum => 'checksum']) ); @@ -242,7 +246,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase public function testResetUniqueField() { $uniqueFields = new \ReflectionProperty( - \Magento\Framework\Model\ResourceModel\Db\AbstractDb::class, + AbstractDb::class, '_uniqueFields' ); $uniqueFields->setAccessible(true); @@ -254,7 +258,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase public function testGetUniqueFields() { $uniqueFieldsReflection = new \ReflectionProperty( - \Magento\Framework\Model\ResourceModel\Db\AbstractDb::class, + AbstractDb::class, '_uniqueFields' ); $uniqueFieldsReflection->setAccessible(true); @@ -281,14 +285,14 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase $this->assertEquals($this->_model, $result); $this->assertInstanceOf( \Magento\Framework\Model\ResourceModel\Db\AbstractDb::class, - $result + $result ); } public function testDelete() { $connectionInterfaceMock = $this->getMock( - \Magento\Framework\DB\Adapter\AdapterInterface::class, + AdapterInterface::class, [], [], '', @@ -297,7 +301,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase $contextMock = $this->getMock(\Magento\Framework\Model\Context::class, [], [], '', false); $registryMock = $this->getMock(\Magento\Framework\Registry::class, [], [], '', false); $abstractModelMock = $this->getMockForAbstractClass( - \Magento\Framework\Model\AbstractModel::class, + AbstractModel::class, [$contextMock, $registryMock], '', false, @@ -311,7 +315,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase ); $abstractModelMock->expects($this->once())->method('getData')->willReturn(['data' => 'value']); - $connectionMock = $this->getMock(\Magento\Framework\DB\Adapter\AdapterInterface::class); + $connectionMock = $this->getMock(AdapterInterface::class); $this->transactionManagerMock->expects($this->once()) ->method('start') ->with($connectionInterfaceMock) @@ -334,13 +338,13 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase $this->returnValue('tableName') ); $mainTableReflection = new \ReflectionProperty( - \Magento\Framework\Model\ResourceModel\Db\AbstractDb::class, + AbstractDb::class, '_mainTable' ); $mainTableReflection->setAccessible(true); $mainTableReflection->setValue($this->_model, 'tableName'); $idFieldNameReflection = new \ReflectionProperty( - \Magento\Framework\Model\ResourceModel\Db\AbstractDb::class, + AbstractDb::class, '_idFieldName' ); $idFieldNameReflection->setAccessible(true); @@ -351,7 +355,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase $abstractModelMock->expects($this->once())->method('afterDelete'); $abstractModelMock->expects($this->once())->method('afterDeleteCommit'); $this->assertInstanceOf( - \Magento\Framework\Model\ResourceModel\Db\AbstractDb::class, + AbstractDb::class, $this->_model->delete($abstractModelMock) ); } @@ -361,7 +365,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase $contextMock = $this->getMock(\Magento\Framework\Model\Context::class, [], [], '', false); $registryMock = $this->getMock(\Magento\Framework\Registry::class, [], [], '', false); $abstractModelMock = $this->getMockForAbstractClass( - \Magento\Framework\Model\AbstractModel::class, + AbstractModel::class, [$contextMock, $registryMock], '', false, @@ -381,7 +385,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase public function testGetDataChanged($getOriginData, $expected) { $connectionInterfaceMock = $this->getMock( - \Magento\Framework\DB\Adapter\AdapterInterface::class, + AdapterInterface::class, [], [], '', @@ -393,7 +397,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase $contextMock = $this->getMock(\Magento\Framework\Model\Context::class, [], [], '', false); $registryMock = $this->getMock(\Magento\Framework\Registry::class, [], [], '', false); $abstractModelMock = $this->getMockForAbstractClass( - \Magento\Framework\Model\AbstractModel::class, + AbstractModel::class, [$contextMock, $registryMock], '', false, @@ -402,7 +406,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase ['__wakeup', 'getOrigData', 'getData'] ); $mainTableProperty = new \ReflectionProperty( - \Magento\Framework\Model\ResourceModel\Db\AbstractDb::class, + AbstractDb::class, '_mainTable' ); $mainTableProperty->setAccessible(true); @@ -431,13 +435,13 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase public function testPrepareDataForUpdate() { - $connectionMock = $this->getMock(\Magento\Framework\DB\Adapter\AdapterInterface::class, [], [], '', false); + $connectionMock = $this->getMock(AdapterInterface::class, [], [], '', false); $context = (new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this))->getObject( \Magento\Framework\Model\Context::class ); $registryMock = $this->getMock(\Magento\Framework\Registry::class, [], [], '', false); $resourceMock = $this->getMock( - \Magento\Framework\Model\ResourceModel\Db\AbstractDb::class, + AbstractDb::class, [ '_construct', 'getConnection', @@ -449,7 +453,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase false ); $connectionInterfaceMock = $this->getMock( - \Magento\Framework\DB\Adapter\AdapterInterface::class, + AdapterInterface::class, [], [], '', @@ -462,7 +466,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase ->disableOriginalConstructor() ->getMockForAbstractClass(); $abstractModelMock = $this->getMockForAbstractClass( - \Magento\Framework\Model\AbstractModel::class, + AbstractModel::class, [$context, $registryMock, $resourceMock, $resourceCollectionMock] ); $data = 'tableName'; @@ -474,13 +478,13 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase $this->returnValue('tableName') ); $mainTableReflection = new \ReflectionProperty( - \Magento\Framework\Model\ResourceModel\Db\AbstractDb::class, + AbstractDb::class, '_mainTable' ); $mainTableReflection->setAccessible(true); $mainTableReflection->setValue($this->_model, 'tableName'); $idFieldNameReflection = new \ReflectionProperty( - \Magento\Framework\Model\ResourceModel\Db\AbstractDb::class, + AbstractDb::class, '_idFieldName' ); $idFieldNameReflection->setAccessible(true); @@ -540,7 +544,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase /** * Mock SUT so as not to test extraneous logic */ - $model = $this->getMockBuilder(\Magento\Framework\Model\ResourceModel\Db\AbstractDb::class) + $model = $this->getMockBuilder(AbstractDb::class) ->disableOriginalConstructor() ->setMethods(['_prepareDataForSave', 'getIdFieldName', 'getConnection', 'getMainTable']) ->getMockForAbstractClass(); @@ -557,7 +561,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase $reflectionProperty->setValue($model, $pkIncrement); // Mocked behavior - $connectionMock = $this->getMockBuilder(\Magento\Framework\DB\Adapter\AdapterInterface::class) + $connectionMock = $this->getMockBuilder(AdapterInterface::class) ->disableOriginalConstructor() ->setMethods(['lastInsertId']) ->getMockForAbstractClass(); @@ -579,7 +583,7 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase // Only set object id if not PK autoincrement $setIdInvokedCount = $pkIncrement ? 1 : 0; - $inputObject = $this->getMockBuilder(\Magento\Framework\Model\AbstractModel::class) + $inputObject = $this->getMockBuilder(AbstractModel::class) ->disableOriginalConstructor() ->getMock(); $inputObject->expects($this->exactly($setIdInvokedCount))->method('setId'); @@ -591,9 +595,37 @@ class AbstractDbTest extends \PHPUnit_Framework_TestCase $reflectionMethod->invokeArgs($model, [$inputObject]); } + /** + * @return array + */ public function saveNewObjectDataProvider() { return [[true], [false]]; } + /** + * @expectedException \Magento\Framework\Exception\AlreadyExistsException + */ + public function testDuplicateExceptionProcessingOnSave() + { + $connection = $this->getMock(AdapterInterface::class); + $connection->expects($this->once())->method('rollback'); + + /** @var AbstractDb|\PHPUnit_Framework_MockObject_MockObject $model */ + $model = $this->getMockBuilder(AbstractDb::class) + ->disableOriginalConstructor() + ->setMethods(['getConnection']) + ->getMockForAbstractClass(); + $model->expects($this->any())->method('getConnection')->willReturn($connection); + + /** @var AbstractModel|\PHPUnit_Framework_MockObject_MockObject $object */ + $object = $this->getMockBuilder(AbstractModel::class) + ->disableOriginalConstructor() + ->getMock(); + $object->expects($this->once())->method('hasDataChanges')->willReturn(true); + $object->expects($this->once())->method('beforeSave')->willThrowException(new DuplicateException()); + $object->expects($this->once())->method('setHasDataChanges')->with(true); + + $model->save($object); + } }