diff --git a/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php b/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php index a9da2a3400de9..c75654138c4e8 100644 --- a/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php +++ b/app/code/Magento/Catalog/Model/Product/Attribute/Backend/Sku.php @@ -11,9 +11,11 @@ * * @author Magento Core Team */ + namespace Magento\Catalog\Model\Product\Attribute\Backend; use Magento\Catalog\Model\Product; +use Magento\Framework\Exception\AlreadyExistsException; class Sku extends \Magento\Eav\Model\Entity\Attribute\Backend\AbstractBackend { @@ -43,6 +45,7 @@ public function __construct(\Magento\Framework\Stdlib\StringUtils $string) * Validate SKU * * @param Product $object + * * @return bool * @throws \Magento\Framework\Exception\LocalizedException * @throws \Magento\Framework\Exception\LocalizedException @@ -52,7 +55,9 @@ public function validate($object) $attrCode = $this->getAttribute()->getAttributeCode(); $value = $object->getData($attrCode); if ($this->getAttribute()->getIsRequired() && strlen($value) === 0) { - throw new \Magento\Framework\Exception\LocalizedException(__('The value of attribute "%1" must be set', $attrCode)); + throw new \Magento\Framework\Exception\LocalizedException( + __('The value of attribute "%1" must be set', $attrCode) + ); } if ($this->string->strlen($object->getSku()) > self::SKU_MAX_LENGTH) { @@ -60,13 +65,43 @@ public function validate($object) __('SKU length should be %1 characters maximum.', self::SKU_MAX_LENGTH) ); } + return true; } + /** + * Check unique SKU for product + * + * @param Product $object + * + * @return void + * + * @throws AlreadyExistsException + */ + private function checkUniqueSku($object) + { + $attribute = $this->getAttribute(); + $entity = $attribute->getEntity(); + $attributeCode = $attribute->getAttributeCode(); + $attributeValue = $object->getData($attributeCode); + while (!$entity->checkAttributeUniqueValue($attribute, $object)) { + throw new AlreadyExistsException( + __( + 'Duplicated %attribute found: %value', + [ + 'attribute' => $attributeCode, + 'value' => $attributeValue + ] + ) + ); + } + } + /** * Generate and set unique SKU to product * * @param Product $object + * * @return void */ protected function _generateUniqueSku($object) @@ -92,11 +127,16 @@ protected function _generateUniqueSku($object) * Make SKU unique before save * * @param Product $object + * * @return $this */ public function beforeSave($object) { - $this->_generateUniqueSku($object); + if (true === $object->getIsDuplicate()) { + $this->_generateUniqueSku($object); + } + $this->checkUniqueSku($object); + return parent::beforeSave($object); } @@ -105,7 +145,9 @@ public function beforeSave($object) * * @param \Magento\Eav\Model\Entity\Attribute\AbstractAttribute $attribute * @param Product $object + * * @return int + * @deprecated */ protected function _getLastSimilarAttributeValueIncrement($attribute, $object) { @@ -125,6 +167,7 @@ protected function _getLastSimilarAttributeValueIncrement($attribute, $object) 1 ); $data = $connection->fetchOne($select, $bind); - return abs((int)str_replace($value, '', $data)); + + return abs((int) str_replace($value, '', $data)); } } diff --git a/app/code/Magento/Catalog/Setup/UpgradeSchema.php b/app/code/Magento/Catalog/Setup/UpgradeSchema.php index 1ccfd7c243320..0f25600d5ce7c 100755 --- a/app/code/Magento/Catalog/Setup/UpgradeSchema.php +++ b/app/code/Magento/Catalog/Setup/UpgradeSchema.php @@ -142,9 +142,26 @@ public function upgrade(SchemaSetupInterface $setup, ModuleContextInterface $con $this->enableSegmentation($setup); } + if (version_compare($context->getVersion(), '2.2.6', '<')) { + $this->modifySkuColumn($setup); + } + $setup->endSetup(); } + /** + * @param SchemaSetupInterface $setup + */ + private function modifySkuColumn(SchemaSetupInterface $setup) + { + $setup->getConnection()->addIndex( + $setup->getTable('catalog_product_entity'), + $setup->getIdxName('catalog_product_entity', ['sku']), + ['sku'], + \Magento\Framework\DB\Adapter\AdapterInterface::INDEX_TYPE_UNIQUE + ); + } + /** * Change definition of customer group id column * diff --git a/app/code/Magento/Catalog/etc/module.xml b/app/code/Magento/Catalog/etc/module.xml index cb27a54c2b58b..23e130aa8a991 100644 --- a/app/code/Magento/Catalog/etc/module.xml +++ b/app/code/Magento/Catalog/etc/module.xml @@ -6,7 +6,7 @@ */ --> - + diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/Backend/SkuTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/Backend/SkuTest.php index f557919897869..357f97c9d27c3 100644 --- a/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/Backend/SkuTest.php +++ b/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/Backend/SkuTest.php @@ -3,10 +3,12 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Catalog\Model\Product\Attribute\Backend; /** * Test class for \Magento\Catalog\Model\Product\Attribute\Backend\Sku. + * * @magentoAppArea adminhtml */ class SkuTest extends \PHPUnit\Framework\TestCase @@ -14,7 +16,28 @@ class SkuTest extends \PHPUnit\Framework\TestCase /** * @magentoDataFixture Magento/Catalog/_files/product_simple.php */ - public function testGenerateUniqueSkuExistingProduct() + public function testGenerateUniqueSkuDuplicatedProduct() + { + $repository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( + \Magento\Catalog\Model\ProductRepository::class + ); + $product = $repository->get('simple'); + /** @var \Magento\Catalog\Model\Product\Copier $copier */ + $copier = $this->getCopier(); + /** @var \Magento\Catalog\Model\Product; $product2 */ + $product2 = $copier->copy($product); + + $this->assertEquals('simple', $product->getSku()); + $product2->getResource()->getAttribute('sku')->getBackend()->beforeSave($product2); + $this->assertEquals('simple-1', $product2->getSku()); + } + + /** + * Checks if generation of unique sku is not allowed + * + * @magentoDataFixture Magento/Catalog/_files/product_simple.php + */ + public function testPreventGenerateUniqueSkuExistingProduct() { $repository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( \Magento\Catalog\Model\ProductRepository::class @@ -22,12 +45,16 @@ public function testGenerateUniqueSkuExistingProduct() $product = $repository->get('simple'); $product->setId(null); $this->assertEquals('simple', $product->getSku()); + + $this->expectException('Magento\Framework\Exception\AlreadyExistsException'); $product->getResource()->getAttribute('sku')->getBackend()->beforeSave($product); - $this->assertEquals('simple-1', $product->getSku()); + + $this->fail('Unique sku generation should be allowed only for product duplication.'); } /** * @param $product \Magento\Catalog\Model\Product + * * @dataProvider uniqueSkuDataProvider */ public function testGenerateUniqueSkuNotExistingProduct($product) @@ -42,22 +69,54 @@ public function testGenerateUniqueSkuNotExistingProduct($product) * @magentoAppArea adminhtml * @magentoDbIsolation enabled */ - public function testGenerateUniqueLongSku() + public function testGenerateUniqueLongSkuDuplicatedProduct() { $repository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( \Magento\Catalog\Model\ProductRepository::class ); $product = $repository->get('simple'); - $product->setSku('0123456789012345678901234567890123456789012345678901234567890123'); + $product->setSku('0123456789012345678901234567890123456789012345678901234567890124'); + $product->save(); + $product->getResource()->getAttribute('sku')->getBackend()->beforeSave($product); + $this->assertEquals('0123456789012345678901234567890123456789012345678901234567890124', $product->getSku()); /** @var \Magento\Catalog\Model\Product\Copier $copier */ - $copier = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get( - \Magento\Catalog\Model\Product\Copier::class + $copier = $this->getCopier(); + $product2 = $copier->copy($product); + + $this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-1', $product2->getSku()); + $product2->getResource()->getAttribute('sku')->getBackend()->beforeSave($product2); + $this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-1', $product2->getSku()); + + $product2->setId(null); + $product2->getResource()->getAttribute('sku')->getBackend()->beforeSave($product2); + $this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-2', $product2->getSku()); + } + + /** + * Checks if generation of long unique sku is not allowed + * + * @magentoDataFixture Magento/Catalog/_files/product_simple.php + * @magentoAppArea adminhtml + * @magentoDbIsolation enabled + */ + public function testPreventGenerateUniqueLongSku() + { + $repository = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( + \Magento\Catalog\Model\ProductRepository::class ); + $product = $repository->get('simple'); + $product->setSku('0123456789012345678901234567890123456789012345678901234567890123'); + + /** @var \Magento\Catalog\Model\Product\Copier $copier */ + $copier = $this->getCopier(); + $copier->copy($product); $this->assertEquals('0123456789012345678901234567890123456789012345678901234567890123', $product->getSku()); + $this->expectException('Magento\Framework\Exception\AlreadyExistsException'); $product->getResource()->getAttribute('sku')->getBackend()->beforeSave($product); - $this->assertEquals('01234567890123456789012345678901234567890123456789012345678901-1', $product->getSku()); + + $this->fail('Unique long sku generation should be allowed only for product duplication.'); } /** @@ -68,6 +127,7 @@ public function testGenerateUniqueLongSku() public function uniqueSkuDataProvider() { $product = $this->_getProduct(); + return [[$product]]; } @@ -107,6 +167,17 @@ protected function _getProduct() )->setStockData( ['use_config_manage_stock' => 1, 'qty' => 100, 'is_qty_decimal' => 0, 'is_in_stock' => 1] ); + return $product; } + + /** + * @return \Magento\Catalog\Model\Product\Copier + */ + protected function getCopier() + { + return \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get( + \Magento\Catalog\Model\Product\Copier::class + ); + } }