ENGCOM-4246: [EAV] Improving the EAV attribute code validation, by not allowing to use n… #20526

Author: sivaschenko

app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Save.php

@@ -195,25 +195,6 @@ public function execute()
                 ? $model->getAttributeCode()
                 : $this->getRequest()->getParam('attribute_code');
             $attributeCode = $attributeCode ?: $this->generateCode($this->getRequest()->getParam('frontend_label')[0]);
-            if (strlen($attributeCode) > 0) {
-                $validatorAttrCode = new \Zend_Validate_Regex(
-                    ['pattern' => '/^[a-zA-Z\x{600}-\x{6FF}][a-zA-Z\x{600}-\x{6FF}_0-9]{0,30}$/u']
-                );
-                if (!$validatorAttrCode->isValid($attributeCode)) {
-                    $this->messageManager->addErrorMessage(
-                        __(
-                            'Attribute code "%1" is invalid. Please use only letters (a-z or A-Z), ' .
-                            'numbers (0-9) or underscore(_) in this field, first character should be a letter.',
-                            $attributeCode
-                        )
-                    );
-                    return $this->returnResult(
-                        'catalog/*/edit',
-                        ['attribute_id' => $attributeId, '_current' => true],
-                        ['error' => true]
-                    );
-                }
-            }
             $data['attribute_code'] = $attributeCode;
 
             //validate frontend_input

app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Validate.php

@@ -7,12 +7,13 @@
 
 namespace Magento\Catalog\Controller\Adminhtml\Product\Attribute;
 
-use Magento\Framework\Serialize\Serializer\FormData;
+use Magento\Catalog\Controller\Adminhtml\Product\Attribute as AttributeAction;
+use Magento\Eav\Model\Validator\Attribute\Code as AttributeCodeValidator;
 use Magento\Framework\App\Action\HttpGetActionInterface;
 use Magento\Framework\App\Action\HttpPostActionInterface as HttpPostActionInterface;
 use Magento\Framework\App\ObjectManager;
 use Magento\Framework\DataObject;
-use Magento\Catalog\Controller\Adminhtml\Product\Attribute as AttributeAction;
+use Magento\Framework\Serialize\Serializer\FormData;
 
 /**
  * Product attribute validate controller.
@@ -43,6 +44,11 @@ class Validate extends AttributeAction implements HttpGetActionInterface, HttpPo
      */
     private $formDataSerializer;
 
+    /**
+     * @var AttributeCodeValidator
+     */
+    private $attributeCodeValidator;
+
     /**
      * Constructor
      *
@@ -54,6 +60,7 @@ class Validate extends AttributeAction implements HttpGetActionInterface, HttpPo
      * @param \Magento\Framework\View\LayoutFactory $layoutFactory
      * @param array $multipleAttributeList
      * @param FormData|null $formDataSerializer
+     * @param AttributeCodeValidator|null $attributeCodeValidator
      */
     public function __construct(
         \Magento\Backend\App\Action\Context $context,
@@ -63,14 +70,18 @@ public function __construct(
         \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory,
         \Magento\Framework\View\LayoutFactory $layoutFactory,
         array $multipleAttributeList = [],
-        FormData $formDataSerializer = null
+        FormData $formDataSerializer = null,
+        AttributeCodeValidator $attributeCodeValidator = null
     ) {
         parent::__construct($context, $attributeLabelCache, $coreRegistry, $resultPageFactory);
         $this->resultJsonFactory = $resultJsonFactory;
         $this->layoutFactory = $layoutFactory;
         $this->multipleAttributeList = $multipleAttributeList;
         $this->formDataSerializer = $formDataSerializer ?: ObjectManager::getInstance()
             ->get(FormData::class);
+        $this->attributeCodeValidator = $attributeCodeValidator ?: ObjectManager::getInstance()->get(
+            AttributeCodeValidator::class
+        );
     }
 
     /**
@@ -115,6 +126,12 @@ public function execute()
             $response->setError(true);
             $response->setProductAttribute($attribute->toArray());
         }
+
+        if (!$this->attributeCodeValidator->isValid($attributeCode)) {
+            $this->setMessageToResponse($response, $this->attributeCodeValidator->getMessages());
+            $response->setError(true);
+        }
+
         if ($this->getRequest()->has('new_attribute_set_name')) {
             $setName = $this->getRequest()->getParam('new_attribute_set_name');
             /** @var $attributeSet \Magento\Eav\Model\Entity\Attribute\Set */

app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/SaveTest.php

@@ -7,6 +7,7 @@
 
 use Magento\Catalog\Api\Data\ProductAttributeInterface;
 use Magento\Catalog\Controller\Adminhtml\Product\Attribute\Save;
+use Magento\Eav\Model\Validator\Attribute\Code as AttributeCodeValidator;
 use Magento\Framework\Serialize\Serializer\FormData;
 use Magento\Catalog\Test\Unit\Controller\Adminhtml\Product\AttributeTest;
 use Magento\Catalog\Model\Product\AttributeSet\BuildFactory;
@@ -94,6 +95,11 @@ class SaveTest extends AttributeTest
      */
     private $productAttributeMock;
 
+    /**
+     * @var AttributeCodeValidator|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $attributeCodeValidatorMock;
+
     protected function setUp()
     {
         parent::setUp();
@@ -138,6 +144,9 @@ protected function setUp()
         $this->formDataSerializerMock = $this->getMockBuilder(FormData::class)
             ->disableOriginalConstructor()
             ->getMock();
+        $this->attributeCodeValidatorMock = $this->getMockBuilder(AttributeCodeValidator::class)
+            ->disableOriginalConstructor()
+            ->getMock();
         $this->productAttributeMock = $this->getMockBuilder(ProductAttributeInterface::class)
             ->setMethods(['getId', 'get'])
             ->getMockForAbstractClass();
@@ -171,6 +180,7 @@ protected function getModel()
             'groupCollectionFactory' => $this->groupCollectionFactoryMock,
             'layoutFactory' => $this->layoutFactoryMock,
             'formDataSerializer' => $this->formDataSerializerMock,
+            'attributeCodeValidator' => $this->attributeCodeValidatorMock
         ]);
     }
 
@@ -224,6 +234,10 @@ public function testExecute()
         $this->productAttributeMock
             ->method('getAttributeCode')
             ->willReturn('test_code');
+        $this->attributeCodeValidatorMock
+            ->method('isValid')
+            ->with('test_code')
+            ->willReturn(true);
         $this->requestMock->expects($this->once())
             ->method('getPostValue')
             ->willReturn($data);

app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/ValidateTest.php

@@ -6,6 +6,7 @@
 namespace Magento\Catalog\Test\Unit\Controller\Adminhtml\Product\Attribute;
 
 use Magento\Catalog\Controller\Adminhtml\Product\Attribute\Validate;
+use Magento\Eav\Model\Validator\Attribute\Code as AttributeCodeValidator;
 use Magento\Framework\Serialize\Serializer\FormData;
 use Magento\Catalog\Model\ResourceModel\Eav\Attribute;
 use Magento\Catalog\Test\Unit\Controller\Adminhtml\Product\AttributeTest;
@@ -67,6 +68,11 @@ class ValidateTest extends AttributeTest
      */
     private $formDataSerializerMock;
 
+    /**
+     * @var AttributeCodeValidator|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $attributeCodeValidatorMock;
+
     protected function setUp()
     {
         parent::setUp();
@@ -95,6 +101,9 @@ protected function setUp()
         $this->formDataSerializerMock = $this->getMockBuilder(FormData::class)
             ->disableOriginalConstructor()
             ->getMock();
+        $this->attributeCodeValidatorMock = $this->getMockBuilder(AttributeCodeValidator::class)
+            ->disableOriginalConstructor()
+            ->getMock();
 
         $this->contextMock->expects($this->any())
             ->method('getObjectManager')
@@ -117,6 +126,7 @@ protected function getModel()
                 'layoutFactory' => $this->layoutFactoryMock,
                 'multipleAttributeList' => ['select' => 'option'],
                 'formDataSerializer' => $this->formDataSerializerMock,
+                'attributeCodeValidator' => $this->attributeCodeValidatorMock,
             ]
         );
     }
@@ -141,6 +151,12 @@ public function testExecute()
         $this->attributeMock->expects($this->once())
             ->method('loadByCode')
             ->willReturnSelf();
+
+        $this->attributeCodeValidatorMock->expects($this->once())
+            ->method('isValid')
+            ->with('test_attribute_code')
+            ->willReturn(true);
+
         $this->requestMock->expects($this->once())
             ->method('has')
             ->with('new_attribute_set_name')
@@ -190,6 +206,11 @@ public function testUniqueValidation(array $options, $isError)
             ->with($serializedOptions)
             ->willReturn($options);
 
+        $this->attributeCodeValidatorMock->expects($this->once())
+            ->method('isValid')
+            ->with('test_attribute_code')
+            ->willReturn(true);
+
         $this->objectManagerMock->expects($this->once())
             ->method('create')
             ->willReturn($this->attributeMock);
@@ -333,6 +354,11 @@ public function testEmptyOption(array $options, $result)
             ->method('loadByCode')
             ->willReturnSelf();
 
+        $this->attributeCodeValidatorMock->expects($this->once())
+            ->method('isValid')
+            ->with('test_attribute_code')
+            ->willReturn(true);
+
         $this->resultJsonFactoryMock->expects($this->once())
             ->method('create')
             ->willReturn($this->resultJson);
@@ -444,6 +470,10 @@ public function testExecuteWithOptionsDataError()
                 [\Magento\Eav\Model\Entity\Attribute\Set::class, [], $this->attributeSetMock]
             ]);
 
+        $this->attributeCodeValidatorMock
+            ->method('isValid')
+            ->willReturn(true);
+
         $this->attributeMock
             ->method('loadByCode')
             ->willReturnSelf();
@@ -463,4 +493,81 @@ public function testExecuteWithOptionsDataError()
 
         $this->getModel()->execute();
     }
+
+    /**
+     * Test execute with an invalid attribute code
+     *
+     * @dataProvider provideInvalidAttributeCodes
+     * @param string $attributeCode
+     * @param $result
+     * @throws \Magento\Framework\Exception\NotFoundException
+     */
+    public function testExecuteWithInvalidAttributeCode($attributeCode, $result)
+    {
+        $serializedOptions = '{"key":"value"}';
+        $this->requestMock->expects($this->any())
+            ->method('getParam')
+            ->willReturnMap([
+                ['frontend_label', null, null],
+                ['frontend_input', 'select', 'multipleselect'],
+                ['attribute_code', null, $attributeCode],
+                ['new_attribute_set_name', null, 'test_attribute_set_name'],
+                ['message_key', Validate::DEFAULT_MESSAGE_KEY, 'message'],
+                ['serialized_options', '[]', $serializedOptions],
+            ]);
+
+        $this->formDataSerializerMock
+            ->expects($this->once())
+            ->method('unserialize')
+            ->with($serializedOptions)
+            ->willReturn(["key" => "value"]);
+
+        $this->objectManagerMock->expects($this->once())
+            ->method('create')
+            ->willReturn($this->attributeMock);
+
+        $this->attributeMock->expects($this->once())
+            ->method('loadByCode')
+            ->willReturnSelf();
+
+        $this->attributeCodeValidatorMock->expects($this->once())
+            ->method('isValid')
+            ->with($attributeCode)
+            ->willReturn(false);
+
+        $this->attributeCodeValidatorMock->expects($this->once())
+            ->method('getMessages')
+            ->willReturn(['Invalid Attribute Code.']);
+
+        $this->resultJsonFactoryMock->expects($this->once())
+            ->method('create')
+            ->willReturn($this->resultJson);
+
+        $this->resultJson->expects($this->once())
+            ->method('setJsonData')
+            ->willReturnArgument(0);
+
+        $response = $this->getModel()->execute();
+        $responseObject = json_decode($response);
+
+        $this->assertEquals($responseObject, $result);
+    }
+
+    /**
+     * Providing invalid attribute codes
+     *
+     * @return array
+     */
+    public function provideInvalidAttributeCodes()
+    {
+        return [
+            'invalid attribute code' => [
+                '.attribute_code',
+                (object) [
+                    'error' => true,
+                    'message' => 'Invalid Attribute Code.',
+                ]
+            ]
+        ];
+    }
 }