Skip to content

Commit 2e374a6

Browse files
author
Volodymyr Kublytskyi
committed
Migration of Message Queue from Magento Commerce to Magento Open Source.
 - fixed code style and code organization issues
1 parent fbbfe65 commit 2e374a6

File tree

5 files changed

+160
-52
lines changed

5 files changed

+160
-52
lines changed

Diff for: ‎app/code/Magento/Backend/Model/Menu/Item/Validator.php

+67-19
Original file line numberDiff line numberDiff line change
@@ -75,37 +75,85 @@ public function __construct()
7575
*/
7676
public function validate($data)
7777
{
78-
if (isset($data['id'], $data['removed']) && $data['removed'] === true) {
78+
if ($this->checkMenuItemIsRemoved($data)) {
7979
return;
8080
}
8181

82+
$this->assertContainsRequiredParameters($data);
83+
$this->assertIdentifierIsNotUsed($data['id']);
84+
85+
foreach ($data as $param => $value) {
86+
$this->validateMenuItemParameter($param, $value);
87+
}
88+
$this->_ids[] = $data['id'];
89+
}
90+
91+
/**
92+
* Check that menu item is not deleted
93+
*
94+
* @param array $data
95+
* @return bool
96+
*/
97+
private function checkMenuItemIsRemoved($data)
98+
{
99+
return isset($data['id'], $data['removed']) && $data['removed'] === true;
100+
}
101+
102+
/**
103+
* Check that menu item contains all required data
104+
* @param array $data
105+
*
106+
* @throws \BadMethodCallException
107+
*/
108+
private function assertContainsRequiredParameters($data)
109+
{
82110
foreach ($this->_required as $param) {
83111
if (!isset($data[$param])) {
84112
throw new \BadMethodCallException('Missing required param ' . $param);
85113
}
86114
}
115+
}
116+
117+
/**
118+
* Check that menu item id is not used
119+
*
120+
* @param string $id
121+
* @throws \InvalidArgumentException
122+
*/
123+
private function assertIdentifierIsNotUsed($id)
124+
{
125+
if (array_search($id, $this->_ids) !== false) {
126+
throw new \InvalidArgumentException('Item with id ' . $id . ' already exists');
127+
}
128+
}
87129

88-
if (array_search($data['id'], $this->_ids) !== false) {
89-
throw new \InvalidArgumentException('Item with id ' . $data['id'] . ' already exists');
130+
/**
131+
* Validate menu item parameter value
132+
*
133+
* @param string $param
134+
* @param mixed $value
135+
* @throws \InvalidArgumentException
136+
*/
137+
private function validateMenuItemParameter($param, $value)
138+
{
139+
if ($value === null) {
140+
return;
141+
}
142+
if (!isset($this->_validators[$param])) {
143+
return;
90144
}
91145

92-
foreach ($data as $param => $value) {
93-
if ($data[$param] !== null
94-
&& isset(
95-
$this->_validators[$param]
96-
) && !$this->_validators[$param]->isValid(
97-
$value
98-
)
99-
) {
100-
throw new \InvalidArgumentException(
101-
"Param " . $param . " doesn't pass validation: " . implode(
102-
'; ',
103-
$this->_validators[$param]->getMessages()
104-
)
105-
);
106-
}
146+
$validator = $this->_validators[$param];
147+
if ($validator->isValid($value)) {
148+
return;
107149
}
108-
$this->_ids[] = $data['id'];
150+
151+
throw new \InvalidArgumentException(
152+
"Param " . $param . " doesn't pass validation: " . implode(
153+
'; ',
154+
$validator->getMessages()
155+
)
156+
);
109157
}
110158

111159
/**

Diff for: ‎dev/tests/static/framework/Magento/TestFramework/Dependency/PhpRule.php

+61-31
Original file line numberDiff line numberDiff line change
@@ -88,47 +88,67 @@ public function getDependencyInfo($currentModule, $fileType, $file, &$contents)
8888
return [];
8989
}
9090

91+
$dependenciesInfo = [];
92+
$dependenciesInfo = $this->considerCaseDependencies(
93+
$dependenciesInfo,
94+
$this->caseClassesAndIdentifiers($currentModule, $file, $contents)
95+
);
96+
$dependenciesInfo = $this->considerCaseDependencies(
97+
$dependenciesInfo,
98+
$this->_caseGetUrl($currentModule, $contents)
99+
);
100+
$dependenciesInfo = $this->considerCaseDependencies(
101+
$dependenciesInfo,
102+
$this->_caseLayoutBlock($currentModule, $fileType, $file, $contents)
103+
);
104+
return $dependenciesInfo;
105+
}
106+
107+
/**
108+
* Check references to classes and identifiers defined in other modules
109+
*
110+
* @param string $currentModule
111+
* @param string $file
112+
* @param string $contents
113+
* @return array
114+
*/
115+
private function caseClassesAndIdentifiers($currentModule, $file, &$contents)
116+
{
91117
$pattern = '~\b(?<class>(?<module>('
92118
. implode(
93119
'_|',
94120
Files::init()->getNamespaces()
95121
)
96122
. '[_\\\\])[a-zA-Z0-9]+)'
97-
. '(?<class_inside_module>[a-zA-Z0-9_\\\\]*))\b(?:::(?<module_scoped_key>[a-z0-9_]+))?~';
123+
. '(?<class_inside_module>[a-zA-Z0-9_\\\\]*))\b(?:::(?<module_scoped_key>[a-z0-9_]+)[\'"])?~';
98124

99-
$dependenciesInfo = [];
100-
if (preg_match_all($pattern, $contents, $matches)) {
101-
$matches['module'] = array_unique($matches['module']);
102-
foreach ($matches['module'] as $i => $referenceModule) {
103-
$referenceModule = str_replace('_', '\\', $referenceModule);
104-
if ($currentModule == $referenceModule) {
105-
continue;
106-
}
125+
if (!preg_match_all($pattern, $contents, $matches)) {
126+
return [];
127+
}
107128

108-
$dependencyClass = trim($matches['class'][$i]);
109-
if (empty($matches['class_inside_module'][$i]) && !empty($matches['module_scoped_key'][$i])) {
110-
$dependencyType = RuleInterface::TYPE_SOFT;
111-
} else {
112-
$currentClass = $this->getClassFromFilepath($file, $currentModule);
113-
$dependencyType = $this->isPluginDependency($currentClass, $dependencyClass)
114-
? RuleInterface::TYPE_SOFT
115-
: RuleInterface::TYPE_HARD;
116-
}
129+
$dependenciesInfo = [];
130+
$matches['module'] = array_unique($matches['module']);
131+
foreach ($matches['module'] as $i => $referenceModule) {
132+
$referenceModule = str_replace('_', '\\', $referenceModule);
133+
if ($currentModule == $referenceModule) {
134+
continue;
135+
}
117136

118-
$dependenciesInfo[] = [
119-
'module' => $referenceModule,
120-
'type' => $dependencyType,
121-
'source' => $dependencyClass,
122-
];
137+
$dependencyClass = trim($matches['class'][$i]);
138+
if (empty($matches['class_inside_module'][$i]) && !empty($matches['module_scoped_key'][$i])) {
139+
$dependencyType = RuleInterface::TYPE_SOFT;
140+
} else {
141+
$currentClass = $this->getClassFromFilepath($file, $currentModule);
142+
$dependencyType = $this->isPluginDependency($currentClass, $dependencyClass)
143+
? RuleInterface::TYPE_SOFT
144+
: RuleInterface::TYPE_HARD;
123145
}
124-
}
125-
$result = $this->_caseGetUrl($currentModule, $contents);
126-
if (count($result)) {
127-
$dependenciesInfo = array_merge($dependenciesInfo, $result);
128-
}
129-
$result = $this->_caseLayoutBlock($currentModule, $fileType, $file, $contents);
130-
if (count($result)) {
131-
$dependenciesInfo = array_merge($dependenciesInfo, $result);
146+
147+
$dependenciesInfo[] = [
148+
'module' => $referenceModule,
149+
'type' => $dependencyType,
150+
'source' => $dependencyClass,
151+
];
132152
}
133153

134154
return $dependenciesInfo;
@@ -374,4 +394,14 @@ protected function _getUniqueDependencies($dependencies = [])
374394
}
375395
return $result;
376396
}
397+
398+
/**
399+
* @param array $known
400+
* @param array $new
401+
* @return array
402+
*/
403+
private function considerCaseDependencies($known, $new)
404+
{
405+
return array_merge($known, $new);
406+
}
377407
}

Diff for: ‎dev/tests/static/testsuite/Magento/Test/Legacy/_files/obsolete_classes.php

+4-1
Original file line numberDiff line numberDiff line change
@@ -4213,7 +4213,10 @@
42134213
['Magento\Framework\Acl\Test\Unit\CacheTest'],
42144214
['Magento\Eav\Model\Entity\Attribute\Backend\Serialized'],
42154215
['Magento\Fmework\MessageQueue\Config\Reader\Xml\Converter\DeprecatedFormat'],
4216-
['Magento\Framework\MessageQueue\Config\Converter', 'Magento\Framework\MessageQueue\Config\Reader\Xml\CompositeConverter'],
4216+
[
4217+
'Magento\Framework\MessageQueue\Config\Converter',
4218+
'Magento\Framework\MessageQueue\Config\Reader\Xml\CompositeConverter'
4219+
],
42174220
['Magento\Framework\MessageQueue\Config\Reader', 'Magento\Framework\MessageQueue\Config\Reader\Xml'],
42184221
['Magento\Framework\MessageQueue\PublisherFactory'],
42194222
['Magento\Framework\MessageQueue\PublisherProxy'],

Diff for: ‎dev/tests/static/testsuite/Magento/Test/Legacy/_files/security/unsecure_php_functions.php

+26-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,32 @@
3939
],
4040
'md5' => [
4141
'replacement' => '',
42-
'exclude' => []
42+
'exclude' => [
43+
/*
44+
* Usage of md5 in MessageQueue key generation algorithm
45+
* added to exclude list to avoid backward incompatible changes
46+
*/
47+
[
48+
'type' => 'library',
49+
'name' => 'magento/framework',
50+
'path' => 'MessageQueue/Rpc/Publisher.php',
51+
],
52+
[
53+
'type' => 'library',
54+
'name' => 'magento/framework',
55+
'path' => 'MessageQueue/MessageController.php',
56+
],
57+
[
58+
'type' => 'library',
59+
'name' => 'magento/framework',
60+
'path' => 'MessageQueue/Publisher.php',
61+
],
62+
[
63+
'type' => 'module',
64+
'name' => 'Magento_AsynchronousOperations',
65+
'path' => 'Model/ResourceModel/System/Message/Collection/Synchronized/Plugin.php'
66+
]
67+
]
4368
],
4469
'srand' => [
4570
'replacement' => '',

Diff for: ‎lib/internal/Magento/Framework/MessageQueue/Rpc/Publisher.php

+2
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class Publisher implements PublisherInterface
4949
*/
5050
private $publisherConfig;
5151

52+
//@codingStandardsIgnoreStart
5253
/**
5354
* Initialize dependencies.
5455
*
@@ -74,6 +75,7 @@ public function __construct(
7475
$this->messageEncoder = $messageEncoder;
7576
$this->messageValidator = $messageValidator;
7677
}
78+
//@codingStandardsIgnoreEnd
7779

7880
/**
7981
* {@inheritdoc}

0 commit comments

Comments
 (0)