Skip to content

Commit 50b78da

Browse files
eduard13eduard13
committed
Refactoring the validator usage. Updating the Unit/Integration tests. Covering the new validator class by Unit Test
1 parent 5666a85 commit 50b78da

File tree

11 files changed

+238
-71
lines changed

11 files changed

+238
-71
lines changed

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

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -195,44 +195,6 @@ public function execute()
195195
? $model->getAttributeCode()
196196
: $this->getRequest()->getParam('attribute_code');
197197
$attributeCode = $attributeCode ?: $this->generateCode($this->getRequest()->getParam('frontend_label')[0]);
198-
if (strlen($attributeCode) > 0) {
199-
$attributeCodeIsValid = true;
200-
$minLength = \Magento\Eav\Model\Entity\Attribute::ATTRIBUTE_CODE_MIN_LENGTH;
201-
$maxLength = \Magento\Eav\Model\Entity\Attribute::ATTRIBUTE_CODE_MAX_LENGTH;
202-
203-
if (strlen($attributeCode) < $minLength || strlen($attributeCode) > $maxLength) {
204-
$this->messageManager->addErrorMessage(
205-
__(
206-
'An attribute code must not be less than %1 and more than %2 characters.',
207-
$minLength,
208-
$maxLength
209-
)
210-
);
211-
$attributeCodeIsValid = false;
212-
}
213-
214-
$validatorAttrCode = new \Zend_Validate_Regex(
215-
['pattern' => '/^[a-zA-Z]+[a-zA-Z0-9_]*$/u']
216-
);
217-
if (!$validatorAttrCode->isValid($attributeCode)) {
218-
$this->messageManager->addErrorMessage(
219-
__(
220-
'Attribute code "%1" is invalid. Please use only letters (a-z or A-Z), ' .
221-
'numbers (0-9) or underscore(_) in this field, first character should be a letter.',
222-
$attributeCode
223-
)
224-
);
225-
$attributeCodeIsValid = false;
226-
}
227-
228-
if (!$attributeCodeIsValid) {
229-
return $this->returnResult(
230-
'catalog/*/edit',
231-
['attribute_id' => $attributeId, '_current' => true],
232-
['error' => true]
233-
);
234-
}
235-
}
236198
$data['attribute_code'] = $attributeCode;
237199

238200
//validate frontend_input

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
namespace Magento\Catalog\Controller\Adminhtml\Product\Attribute;
99

10+
use Magento\Eav\Model\Validator\Attribute\Code as AttributeCodeValidator;
1011
use Magento\Framework\Serialize\Serializer\FormData;
1112
use Magento\Framework\App\Action\HttpGetActionInterface;
1213
use Magento\Framework\App\Action\HttpPostActionInterface as HttpPostActionInterface;
@@ -43,6 +44,11 @@ class Validate extends AttributeAction implements HttpGetActionInterface, HttpPo
4344
*/
4445
private $formDataSerializer;
4546

47+
/**
48+
* @var AttributeCodeValidator
49+
*/
50+
private $attributeCodeValidator;
51+
4652
/**
4753
* Constructor
4854
*
@@ -54,6 +60,7 @@ class Validate extends AttributeAction implements HttpGetActionInterface, HttpPo
5460
* @param \Magento\Framework\View\LayoutFactory $layoutFactory
5561
* @param array $multipleAttributeList
5662
* @param FormData|null $formDataSerializer
63+
* @param AttributeCodeValidator|null $attributeCodeValidator
5764
*/
5865
public function __construct(
5966
\Magento\Backend\App\Action\Context $context,
@@ -63,14 +70,17 @@ public function __construct(
6370
\Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory,
6471
\Magento\Framework\View\LayoutFactory $layoutFactory,
6572
array $multipleAttributeList = [],
66-
FormData $formDataSerializer = null
73+
FormData $formDataSerializer = null,
74+
AttributeCodeValidator $attributeCodeValidator = null
6775
) {
6876
parent::__construct($context, $attributeLabelCache, $coreRegistry, $resultPageFactory);
6977
$this->resultJsonFactory = $resultJsonFactory;
7078
$this->layoutFactory = $layoutFactory;
7179
$this->multipleAttributeList = $multipleAttributeList;
7280
$this->formDataSerializer = $formDataSerializer ?: ObjectManager::getInstance()
7381
->get(FormData::class);
82+
$this->attributeCodeValidator = $attributeCodeValidator ?: ObjectManager::getInstance()->get(
83+
AttributeCodeValidator::class);
7484
}
7585

7686
/**
@@ -115,6 +125,12 @@ public function execute()
115125
$response->setError(true);
116126
$response->setProductAttribute($attribute->toArray());
117127
}
128+
129+
if (!$this->attributeCodeValidator->isValid($attributeCode)) {
130+
$this->setMessageToResponse($response, $this->attributeCodeValidator->getMessages());
131+
$response->setError(true);
132+
}
133+
118134
if ($this->getRequest()->has('new_attribute_set_name')) {
119135
$setName = $this->getRequest()->getParam('new_attribute_set_name');
120136
/** @var $attributeSet \Magento\Eav\Model\Entity\Attribute\Set */

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
use Magento\Catalog\Api\Data\ProductAttributeInterface;
99
use Magento\Catalog\Controller\Adminhtml\Product\Attribute\Save;
10+
use Magento\Eav\Model\Validator\Attribute\Code as AttributeCodeValidator;
1011
use Magento\Framework\Serialize\Serializer\FormData;
1112
use Magento\Catalog\Test\Unit\Controller\Adminhtml\Product\AttributeTest;
1213
use Magento\Catalog\Model\Product\AttributeSet\BuildFactory;
@@ -94,6 +95,11 @@ class SaveTest extends AttributeTest
9495
*/
9596
private $productAttributeMock;
9697

98+
/**
99+
* @var AttributeCodeValidator|\PHPUnit_Framework_MockObject_MockObject
100+
*/
101+
private $attributeCodeValidatorMock;
102+
97103
protected function setUp()
98104
{
99105
parent::setUp();
@@ -138,6 +144,9 @@ protected function setUp()
138144
$this->formDataSerializerMock = $this->getMockBuilder(FormData::class)
139145
->disableOriginalConstructor()
140146
->getMock();
147+
$this->attributeCodeValidatorMock = $this->getMockBuilder(AttributeCodeValidator::class)
148+
->disableOriginalConstructor()
149+
->getMock();
141150
$this->productAttributeMock = $this->getMockBuilder(ProductAttributeInterface::class)
142151
->setMethods(['getId', 'get'])
143152
->getMockForAbstractClass();
@@ -171,6 +180,7 @@ protected function getModel()
171180
'groupCollectionFactory' => $this->groupCollectionFactoryMock,
172181
'layoutFactory' => $this->layoutFactoryMock,
173182
'formDataSerializer' => $this->formDataSerializerMock,
183+
'attributeCodeValidator' => $this->attributeCodeValidatorMock
174184
]);
175185
}
176186

@@ -224,6 +234,10 @@ public function testExecute()
224234
$this->productAttributeMock
225235
->method('getAttributeCode')
226236
->willReturn('test_code');
237+
$this->attributeCodeValidatorMock
238+
->method('isValid')
239+
->with('test_code')
240+
->willReturn(true);
227241
$this->requestMock->expects($this->once())
228242
->method('getPostValue')
229243
->willReturn($data);

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

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
namespace Magento\Catalog\Test\Unit\Controller\Adminhtml\Product\Attribute;
77

88
use Magento\Catalog\Controller\Adminhtml\Product\Attribute\Validate;
9+
use Magento\Eav\Model\Validator\Attribute\Code as AttributeCodeValidator;
910
use Magento\Framework\Serialize\Serializer\FormData;
1011
use Magento\Catalog\Model\ResourceModel\Eav\Attribute;
1112
use Magento\Catalog\Test\Unit\Controller\Adminhtml\Product\AttributeTest;
@@ -67,6 +68,11 @@ class ValidateTest extends AttributeTest
6768
*/
6869
private $formDataSerializerMock;
6970

71+
/**
72+
* @var AttributeCodeValidator|\PHPUnit_Framework_MockObject_MockObject
73+
*/
74+
private $attributeCodeValidatorMock;
75+
7076
protected function setUp()
7177
{
7278
parent::setUp();
@@ -95,6 +101,9 @@ protected function setUp()
95101
$this->formDataSerializerMock = $this->getMockBuilder(FormData::class)
96102
->disableOriginalConstructor()
97103
->getMock();
104+
$this->attributeCodeValidatorMock = $this->getMockBuilder(AttributeCodeValidator::class)
105+
->disableOriginalConstructor()
106+
->getMock();
98107

99108
$this->contextMock->expects($this->any())
100109
->method('getObjectManager')
@@ -117,6 +126,7 @@ protected function getModel()
117126
'layoutFactory' => $this->layoutFactoryMock,
118127
'multipleAttributeList' => ['select' => 'option'],
119128
'formDataSerializer' => $this->formDataSerializerMock,
129+
'attributeCodeValidator' => $this->attributeCodeValidatorMock,
120130
]
121131
);
122132
}
@@ -141,6 +151,12 @@ public function testExecute()
141151
$this->attributeMock->expects($this->once())
142152
->method('loadByCode')
143153
->willReturnSelf();
154+
155+
$this->attributeCodeValidatorMock->expects($this->once())
156+
->method('isValid')
157+
->with('test_attribute_code')
158+
->willReturn(true);
159+
144160
$this->requestMock->expects($this->once())
145161
->method('has')
146162
->with('new_attribute_set_name')
@@ -190,6 +206,11 @@ public function testUniqueValidation(array $options, $isError)
190206
->with($serializedOptions)
191207
->willReturn($options);
192208

209+
$this->attributeCodeValidatorMock->expects($this->once())
210+
->method('isValid')
211+
->with('test_attribute_code')
212+
->willReturn(true);
213+
193214
$this->objectManagerMock->expects($this->once())
194215
->method('create')
195216
->willReturn($this->attributeMock);
@@ -333,6 +354,11 @@ public function testEmptyOption(array $options, $result)
333354
->method('loadByCode')
334355
->willReturnSelf();
335356

357+
$this->attributeCodeValidatorMock->expects($this->once())
358+
->method('isValid')
359+
->with('test_attribute_code')
360+
->willReturn(true);
361+
336362
$this->resultJsonFactoryMock->expects($this->once())
337363
->method('create')
338364
->willReturn($this->resultJson);
@@ -444,6 +470,10 @@ public function testExecuteWithOptionsDataError()
444470
[\Magento\Eav\Model\Entity\Attribute\Set::class, [], $this->attributeSetMock]
445471
]);
446472

473+
$this->attributeCodeValidatorMock
474+
->method('isValid')
475+
->willReturn(true);
476+
447477
$this->attributeMock
448478
->method('loadByCode')
449479
->willReturnSelf();
@@ -463,4 +493,81 @@ public function testExecuteWithOptionsDataError()
463493

464494
$this->getModel()->execute();
465495
}
496+
497+
/**
498+
* Test execute with an invalid attribute code
499+
*
500+
* @dataProvider provideInvalidAttributeCodes
501+
* @param string $attributeCode
502+
* @param $result
503+
* @throws \Magento\Framework\Exception\NotFoundException
504+
*/
505+
public function testExecuteWithInvalidAttributeCode($attributeCode, $result)
506+
{
507+
$serializedOptions = '{"key":"value"}';
508+
$this->requestMock->expects($this->any())
509+
->method('getParam')
510+
->willReturnMap([
511+
['frontend_label', null, null],
512+
['frontend_input', 'select', 'multipleselect'],
513+
['attribute_code', null, $attributeCode],
514+
['new_attribute_set_name', null, 'test_attribute_set_name'],
515+
['message_key', Validate::DEFAULT_MESSAGE_KEY, 'message'],
516+
['serialized_options', '[]', $serializedOptions],
517+
]);
518+
519+
$this->formDataSerializerMock
520+
->expects($this->once())
521+
->method('unserialize')
522+
->with($serializedOptions)
523+
->willReturn(["key" => "value"]);
524+
525+
$this->objectManagerMock->expects($this->once())
526+
->method('create')
527+
->willReturn($this->attributeMock);
528+
529+
$this->attributeMock->expects($this->once())
530+
->method('loadByCode')
531+
->willReturnSelf();
532+
533+
$this->attributeCodeValidatorMock->expects($this->once())
534+
->method('isValid')
535+
->with($attributeCode)
536+
->willReturn(false);
537+
538+
$this->attributeCodeValidatorMock->expects($this->once())
539+
->method('getMessages')
540+
->willReturn(['Invalid Attribute Code.']);
541+
542+
$this->resultJsonFactoryMock->expects($this->once())
543+
->method('create')
544+
->willReturn($this->resultJson);
545+
546+
$this->resultJson->expects($this->once())
547+
->method('setJsonData')
548+
->willReturnArgument(0);
549+
550+
$response = $this->getModel()->execute();
551+
$responseObject = json_decode($response);
552+
553+
$this->assertEquals($responseObject, $result);
554+
}
555+
556+
/**
557+
* Providing invalid attribute codes
558+
*
559+
* @return array
560+
*/
561+
public function provideInvalidAttributeCodes()
562+
{
563+
return [
564+
'invalid attribute code' => [
565+
'.attribute_code',
566+
(object) [
567+
'error' => true,
568+
'message' => 'Invalid Attribute Code.',
569+
]
570+
]
571+
];
572+
}
466573
}

‎app/code/Magento/Eav/Model/Entity/Attribute.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,11 @@ public function loadEntityAttributeIdBySet()
242242
*/
243243
public function beforeSave()
244244
{
245-
if (isset($this->_data['attribute_code'])) {
246-
$this->attributeCodeValidator->isValid($this->_data['attribute_code']);
245+
if (isset($this->_data['attribute_code'])
246+
&& !$this->attributeCodeValidator->isValid($this->_data['attribute_code'])
247+
) {
248+
$errorMessages = implode("\n", $this->attributeCodeValidator->getMessages());
249+
throw new LocalizedException(__($errorMessages));
247250
}
248251

249252
// prevent overriding product data

‎app/code/Magento/Eav/Model/Validator/Attribute/Code.php

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,29 +17,33 @@
1717
*
1818
* Validation EAV attribute code
1919
*/
20-
class Code extends AbstractValidator
20+
class Code extends AbstractValidator
2121
{
22+
/**
23+
* Validation pattern for attribute code
24+
*/
25+
const VALIDATION_RULE_PATTERN = '/^[a-zA-Z]+[a-zA-Z0-9_]*$/u';
26+
2227
/**
2328
* Validates the correctness of the attribute code
2429
*
2530
* @param string $attributeCode
2631
* @return bool
27-
* @throws LocalizedException
32+
* @throws \Zend_Validate_Exception
2833
*/
2934
public function isValid($attributeCode): bool
3035
{
36+
$errorMessages = [];
3137
/**
3238
* Check attribute_code for allowed characters
3339
*/
3440
if (trim($attributeCode)
35-
&& !preg_match('/^[a-zA-Z]+[a-zA-Z0-9_]*$/', trim($attributeCode))
41+
&& !preg_match(self::VALIDATION_RULE_PATTERN, trim($attributeCode))
3642
) {
37-
throw new LocalizedException(
38-
__(
39-
'Attribute code "%1" is invalid. Please use only letters (a-z), ' .
40-
'numbers (0-9) or underscore(_) in this field, first character should be a letter.',
41-
$attributeCode
42-
)
43+
$errorMessages[] = __(
44+
'Attribute code "%1" is invalid. Please use only letters (a-z or A-Z), ' .
45+
'numbers (0-9) or underscore (_) in this field, and the first character should be a letter.',
46+
$attributeCode
4347
);
4448
}
4549

@@ -54,13 +58,15 @@ public function isValid($attributeCode): bool
5458
['min' => $minLength, 'max' => $maxLength]
5559
);
5660
if (!$isAllowedLength) {
57-
throw new LocalizedException(__(
61+
$errorMessages[] = __(
5862
'An attribute code must not be less than %1 and more than %2 characters.',
5963
$minLength,
6064
$maxLength
61-
));
65+
);
6266
}
6367

64-
return true;
68+
$this->_addMessages($errorMessages);
69+
70+
return !$this->hasMessages();
6571
}
6672
}

0 commit comments

Comments
 (0)