Improving the eav attribute code validation, by not allowing to use n…

…ot allowed chars
magento · Jan 23, 2019 · 6f83604 · 6f83604
1 parent 470fd45
commit 6f83604
Show file tree

Hide file tree

Showing 4 changed files with 122 additions and 41 deletions.
diff --git a/app/code/Magento/Eav/Model/Entity/Attribute.php b/app/code/Magento/Eav/Model/Entity/Attribute.php
@@ -5,7 +5,9 @@
  */
 namespace Magento\Eav\Model\Entity;
 
+use Magento\Eav\Model\Validator\Attribute\Code;
 use Magento\Framework\Api\AttributeValueFactory;
+use Magento\Framework\App\ObjectManager;
 use Magento\Framework\Exception\LocalizedException;
 use Magento\Framework\Stdlib\DateTime;
 use Magento\Framework\Stdlib\DateTime\DateTimeFormatterInterface;
@@ -80,6 +82,11 @@ class Attribute extends \Magento\Eav\Model\Entity\Attribute\AbstractAttribute im
      */
     protected $dateTimeFormatter;
 
+    /**
+     * @var Code|null
+     */
+    private $attributeCodeValidator;
+
     /**
      * @param \Magento\Framework\Model\Context $context
      * @param \Magento\Framework\Registry $registry
@@ -99,6 +106,7 @@ class Attribute extends \Magento\Eav\Model\Entity\Attribute\AbstractAttribute im
      * @param DateTimeFormatterInterface $dateTimeFormatter
      * @param \Magento\Framework\Model\ResourceModel\AbstractResource $resource
      * @param \Magento\Framework\Data\Collection\AbstractDb $resourceCollection
+     * @param Code|null $attributeCodeValidator
      * @param array $data
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      * @codeCoverageIgnore
@@ -122,7 +130,8 @@ public function __construct(
         DateTimeFormatterInterface $dateTimeFormatter,
         \Magento\Framework\Model\ResourceModel\AbstractResource $resource = null,
         \Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
-        array $data = []
+        array $data = [],
+        Code $attributeCodeValidator = null
     ) {
         parent::__construct(
             $context,
@@ -145,6 +154,9 @@ public function __construct(
         $this->_localeResolver = $localeResolver;
         $this->reservedAttributeList = $reservedAttributeList;
         $this->dateTimeFormatter = $dateTimeFormatter;
+        $this->attributeCodeValidator = $attributeCodeValidator ?: ObjectManager::getInstance()->get(
+            Code::class
+        );
     }
 
     /**
@@ -230,6 +242,10 @@ public function loadEntityAttributeIdBySet()
      */
     public function beforeSave()
     {
+        if (isset($this->_data['attribute_code'])) {
+            $this->attributeCodeValidator->isValid($this->_data['attribute_code']);
+        }
+
         // prevent overriding product data
         if (isset($this->_data['attribute_code']) && $this->reservedAttributeList->isReservedAttribute($this)) {
             throw new LocalizedException(
@@ -240,25 +256,6 @@ public function beforeSave()
             );
         }
 
-        /**
-         * Check for maximum attribute_code length
-         */
-        if (isset(
-            $this->_data['attribute_code']
-        ) && !\Zend_Validate::is(
-            $this->_data['attribute_code'],
-            'StringLength',
-            ['max' => self::ATTRIBUTE_CODE_MAX_LENGTH]
-        )
-        ) {
-            throw new LocalizedException(
-                __(
-                    'The attribute code needs to be %1 characters or fewer. Re-enter the code and try again.',
-                    self::ATTRIBUTE_CODE_MAX_LENGTH
-                )
-            );
-        }
-
         $defaultValue = $this->getDefaultValue();
         $hasDefaultValue = (string)$defaultValue != '';
 
@@ -513,7 +510,7 @@ public function __sleep()
     public function __wakeup()
     {
         parent::__wakeup();
-        $objectManager = \Magento\Framework\App\ObjectManager::getInstance();
+        $objectManager = ObjectManager::getInstance();
         $this->_localeDate = $objectManager->get(\Magento\Framework\Stdlib\DateTime\TimezoneInterface::class);
         $this->_localeResolver = $objectManager->get(\Magento\Framework\Locale\ResolverInterface::class);
         $this->reservedAttributeList = $objectManager->get(\Magento\Catalog\Model\Product\ReservedAttributeList::class);

diff --git a/app/code/Magento/Eav/Model/Validator/Attribute/Code.php b/app/code/Magento/Eav/Model/Validator/Attribute/Code.php
@@ -0,0 +1,63 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\Eav\Model\Validator\Attribute;
+
+use Magento\Eav\Model\Entity\Attribute;
+use Magento\Framework\Exception\LocalizedException;
+
+/**
+ * Class Code
+ *
+ * Validation EAV attribute code
+ */
+class Code  extends \Magento\Framework\Validator\AbstractValidator
+{
+    /**
+     * Validates the correctness of the attribute code
+     *
+     * @param $attributeCode
+     * @return bool
+     * @throws LocalizedException
+     */
+    public function isValid($attributeCode)
+    {
+        /**
+         * Check attribute_code for allowed characters
+         */
+        if (trim($attributeCode)
+            && !preg_match('/^[a-z][a-z0-9_]*$/', trim($attributeCode))
+        ) {
+            throw new LocalizedException(
+                __(
+                    'Attribute code "%1" is invalid. Please use only letters (a-z), ' .
+                    'numbers (0-9) or underscore(_) in this field, first character should be a letter.',
+                    $attributeCode
+                )
+            );
+        }
+
+        /**
+         * Check attribute_code for allowed length
+         */
+        $minLength = Attribute::ATTRIBUTE_CODE_MIN_LENGTH;
+        $maxLength = Attribute::ATTRIBUTE_CODE_MAX_LENGTH;
+        $isAllowedLength = \Zend_Validate::is(
+            trim($attributeCode),
+            'StringLength',
+            ['min' => $minLength, 'max' => $maxLength]
+        );
+        if (!$isAllowedLength) {
+            throw new LocalizedException(__(
+                'An attribute code must not be less than %1 and more than %2 characters.',
+                $minLength,
+                $maxLength
+            ));
+        }
+
+        return true;
+    }
+}
diff --git a/app/code/Magento/Eav/Setup/EavSetup.php b/app/code/Magento/Eav/Setup/EavSetup.php
@@ -9,6 +9,7 @@
 use Magento\Eav\Model\Entity\Setup\Context;
 use Magento\Eav\Model\Entity\Setup\PropertyMapperInterface;
 use Magento\Eav\Model\ResourceModel\Entity\Attribute\Group\CollectionFactory;
+use Magento\Eav\Model\Validator\Attribute\Code;
 use Magento\Framework\App\CacheInterface;
 use Magento\Framework\App\ObjectManager;
 use Magento\Framework\App\ResourceConnection;
@@ -80,24 +81,34 @@ class EavSetup
      */
     private $_defaultAttributeSetName = 'Default';
 
+    /**
+     * @var Code
+     */
+    private $attributeCodeValidator;
+
     /**
      * Init
      *
      * @param ModuleDataSetupInterface $setup
      * @param Context $context
      * @param CacheInterface $cache
      * @param CollectionFactory $attrGroupCollectionFactory
+     * @param Code|null $attributeCodeValidator
      */
     public function __construct(
         ModuleDataSetupInterface $setup,
         Context $context,
         CacheInterface $cache,
-        CollectionFactory $attrGroupCollectionFactory
+        CollectionFactory $attrGroupCollectionFactory,
+        Code $attributeCodeValidator = null
     ) {
         $this->cache = $cache;
         $this->attrGroupCollectionFactory = $attrGroupCollectionFactory;
         $this->attributeMapper = $context->getAttributeMapper();
         $this->setup = $setup;
+        $this->attributeCodeValidator = $attributeCodeValidator ?: ObjectManager::getInstance()->get(
+            Code::class
+        );
     }
 
     /**
@@ -783,25 +794,8 @@ private function _getValue($array, $key, $default = null)
      */
     private function _validateAttributeData($data)
     {
-        $minLength = \Magento\Eav\Model\Entity\Attribute::ATTRIBUTE_CODE_MIN_LENGTH;
-        $maxLength = \Magento\Eav\Model\Entity\Attribute::ATTRIBUTE_CODE_MAX_LENGTH;
         $attributeCode = isset($data['attribute_code']) ? $data['attribute_code'] : '';
-
-        $isAllowedLength = \Zend_Validate::is(
-            trim($attributeCode),
-            'StringLength',
-            ['min' => $minLength, 'max' => $maxLength]
-        );
-
-        if (!$isAllowedLength) {
-            $errorMessage = __(
-                'An attribute code must not be less than %1 and more than %2 characters.',
-                $minLength,
-                $maxLength
-            );
-
-            throw new LocalizedException($errorMessage);
-        }
+        $this->attributeCodeValidator->isValid($attributeCode);
 
         return true;
     }

diff --git a/dev/tests/integration/testsuite/Magento/Eav/Setup/EavSetupTest.php b/dev/tests/integration/testsuite/Magento/Eav/Setup/EavSetupTest.php
@@ -55,7 +55,7 @@ public function addAttributeDataProvider()
     {
         return [
             ['eav_setup_test'],
-            ['_59_characters_59_characters_59_characters_59_characters_59'],
+            ['characters_59_characters_59_characters_59_characters_59_59_'],
         ];
     }
 
@@ -90,6 +90,33 @@ public function addAttributeThrowExceptionDataProvider()
         ];
     }
 
+    /**
+     * Verify that add attribute throw exception if attribute_code is not valid.
+     *
+     * @param string|null $attributeCode
+     *
+     * @dataProvider addInvalidAttributeThrowExceptionDataProvider
+     * @expectedException \Magento\Framework\Exception\LocalizedException
+     * @expectedExceptionMessage Please use only letters (a-z), numbers (0-9) or underscore(_) in this field, first char
+     */
+    public function testAddInvalidAttributeThrowException($attributeCode)
+    {
+        $attributeData = $this->getAttributeData();
+        $this->eavSetup->addAttribute(\Magento\Catalog\Model\Product::ENTITY, $attributeCode, $attributeData);
+    }
+    /**
+     * Data provider for testAddInvalidAttributeThrowException().
+     *
+     * @return array
+     */
+    public function addInvalidAttributeThrowExceptionDataProvider()
+    {
+        return [
+            ['1first_character_is_not_letter'],
+            ['attribute.with.dots'],
+        ];
+    }
+
     /**
      * Get simple attribute data.
      */