Skip to content

Commit 61a75dd

Browse files
Rrego6Rrego6
committed
MC-24057: Implement static dependency analysis for wildcard and API urls
- Added unit tests
1 parent 0444ee9 commit 61a75dd

File tree

3 files changed

+96
-29
lines changed

3 files changed

+96
-29
lines changed

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
use Magento\TestFramework\Dependency\Reader\ClassScanner;
1717
use Magento\TestFramework\Dependency\Route\RouteMapper;
1818
use Magento\TestFramework\Exception\NoSuchActionException;
19-
use Magento\Webapi\Model\Config\Converter;
19+
use Magento\Test\Integrity\Dependency\Converter;
2020
use PHPUnit\Framework\Exception;
2121

2222
/**
@@ -324,7 +324,7 @@ protected function _caseGetUrl(string $currentModule, string &$contents, string
324324
$returnedDependencies = [];
325325
if (strpos($path, '*') !== false) {
326326
$returnedDependencies = $this->processWildcardUrl($path, $file);
327-
} elseif (preg_match('#rest(?<service>/V1/\w+)#i', $path, $apiMatch)) {
327+
} elseif (preg_match('#rest(?<service>/V1/.+)#i', $path, $apiMatch)) {
328328
$returnedDependencies = $this->processApiUrl($apiMatch['service']);
329329
} else {
330330
$returnedDependencies = $this->processStandardUrl($path);
@@ -365,7 +365,7 @@ private function processWildcardUrl(string $urlPath, string $filePath)
365365
}
366366
$filePathInfo = pathinfo($filePath);
367367
$fileActionName = $filePathInfo['filename'];
368-
$filePathPieces = explode('/', $filePathInfo['dirname']);
368+
$filePathPieces = explode(DIRECTORY_SEPARATOR, $filePathInfo['dirname']);
369369

370370
/**
371371
* Only handle Controllers. ie: Ignore Blocks, Templates, and Models due to complexity in static resolution
@@ -377,8 +377,10 @@ private function processWildcardUrl(string $urlPath, string $filePath)
377377
) {
378378
return [];
379379
}
380-
$fileControllerIndex = array_search('adminhtml', $filePathPieces)
381-
?? array_search('controller', $filePathPieces);
380+
$fileControllerIndex = array_search('adminhtml', $filePathPieces);
381+
if (!$fileControllerIndex) {
382+
$fileControllerIndex = array_search('controller', $filePathPieces);
383+
}
382384

383385
$controllerName = array_shift($urlRoutePieces);
384386
if ('*' === $controllerName) {
@@ -445,22 +447,21 @@ private function processApiUrl(string $path): array
445447
*/
446448
if (!$this->serviceMethods) {
447449
$this->serviceMethods = [];
448-
$serviceRoutes = $this->configReader->read()[Converter::KEY_ROUTES];
450+
$serviceRoutes = $this->configReader->read()['routes'];
449451
foreach ($serviceRoutes as $serviceRouteUrl => $methods) {
450452
$pattern = '#:\w+#';
451453
$replace = '\w';
452-
$serviceRouteUrlRegex = $serviceRouteUrl;
453-
preg_replace($pattern, $replace, $serviceRouteUrlRegex);
454+
$serviceRouteUrlRegex = preg_replace($pattern, $replace, $serviceRouteUrl);
454455
$serviceRouteUrlRegex = '#' . $serviceRouteUrlRegex . '#';
455456
$this->serviceMethods[$serviceRouteUrlRegex] = $methods;
456457
}
457458
}
458-
foreach ($this->serviceMethods as $serviceMethodUrlRegex => $methods) {
459+
foreach ($this->serviceMethods as $serviceRouteUrlRegex => $methods) {
459460
/**
460461
* Since we expect that every service method should be within the same module, we can use the class from
461462
* any method
462463
*/
463-
if (preg_match($serviceMethodUrlRegex, $path)) {
464+
if (preg_match($serviceRouteUrlRegex, $path)) {
464465
$method = reset($methods);
465466

466467
$className = $method['service']['class'];

‎dev/tests/static/framework/tests/unit/testsuite/Magento/TestFramework/Dependency/PhpRuleTest.php

Lines changed: 76 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,13 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\TestFramework\Dependency;
78

9+
use Magento\Framework\Config\Reader\Filesystem;
810
use Magento\Framework\Exception\LocalizedException;
911
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
1012
use Magento\TestFramework\Dependency\Reader\ClassScanner;
11-
use \Magento\Webapi\Model\Config\Reader;
1213

1314
/**
1415
* Test for PhpRule dependency check
@@ -31,7 +32,7 @@ class PhpRuleTest extends \PHPUnit\Framework\TestCase
3132
private $classScanner;
3233

3334
/**
34-
* @var Reader
35+
* @var \PHPUnit\Framework\MockObject\MockObject | Filesystem
3536
*/
3637
private $webApiConfigReader;
3738

@@ -51,15 +52,15 @@ protected function setUp(): void
5152

5253
$this->objectManagerHelper = new ObjectManagerHelper($this);
5354
$this->classScanner = $this->createMock(ClassScanner::class);
54-
$this->webApiConfigReader = $this->objectManagerHelper->getObject(Reader::class);
55+
$this->webApiConfigReader = $this->makeWebApiConfigReaderMock();
5556

5657
$this->model = $this->objectManagerHelper->getObject(
5758
PhpRule::class,
5859
[
5960
'mapRouters' => $mapRoutes,
6061
'mapLayoutBlocks' => $mapLayoutBlocks,
6162
'pluginMap' => $pluginMap,
62-
'webApiConfig' => $this->webApiConfigReader,
63+
'configReader' => $this->webApiConfigReader,
6364
'whitelists' => $whitelist,
6465
'classScanner' => $this->classScanner
6566
]
@@ -114,7 +115,7 @@ public function getDependencyInfoDataProvider()
114115
[
115116
[
116117
'modules' => ['Magento\SomeModule'],
117-
'type' => \Magento\TestFramework\Dependency\RuleInterface::TYPE_HARD,
118+
'type' => RuleInterface::TYPE_HARD,
118119
'source' => 'Magento\SomeModule\Any\ClassName',
119120
]
120121
]
@@ -132,7 +133,7 @@ public function getDependencyInfoDataProvider()
132133
[
133134
[
134135
'modules' => ['Magento\SomeModule'],
135-
'type' => \Magento\TestFramework\Dependency\RuleInterface::TYPE_HARD,
136+
'type' => RuleInterface::TYPE_HARD,
136137
'source' => 'Magento_SomeModule',
137138
]
138139
]
@@ -150,7 +151,7 @@ public function getDependencyInfoDataProvider()
150151
[
151152
[
152153
'modules' => ['Magento\SomeModule'],
153-
'type' => \Magento\TestFramework\Dependency\RuleInterface::TYPE_HARD,
154+
'type' => RuleInterface::TYPE_HARD,
154155
'source' => 'Magento\SomeModule\Any\ClassName',
155156
]
156157
]
@@ -168,7 +169,7 @@ public function getDependencyInfoDataProvider()
168169
[
169170
[
170171
'modules' => ['Magento\SomeModule'],
171-
'type' => \Magento\TestFramework\Dependency\RuleInterface::TYPE_HARD,
172+
'type' => RuleInterface::TYPE_HARD,
172173
'source' => 'getBlock(\'block.name\')',
173174
]
174175
]
@@ -192,7 +193,7 @@ public function getDependencyInfoDataProvider()
192193
[
193194
[
194195
'modules' => ['Magento\Module2'],
195-
'type' => \Magento\TestFramework\Dependency\RuleInterface::TYPE_SOFT,
196+
'type' => RuleInterface::TYPE_SOFT,
196197
'source' => 'Magento\Module2\Subject',
197198
]
198199
],
@@ -204,7 +205,7 @@ public function getDependencyInfoDataProvider()
204205
[
205206
[
206207
'modules' => ['Magento\Module2'],
207-
'type' => \Magento\TestFramework\Dependency\RuleInterface::TYPE_SOFT,
208+
'type' => RuleInterface::TYPE_SOFT,
208209
'source' => 'Magento\Module2\NotSubject',
209210
]
210211
]
@@ -216,7 +217,7 @@ public function getDependencyInfoDataProvider()
216217
[
217218
[
218219
'modules' => ['Magento\OtherModule'],
219-
'type' => \Magento\TestFramework\Dependency\RuleInterface::TYPE_HARD,
220+
'type' => RuleInterface::TYPE_HARD,
220221
'source' => 'Magento\OtherModule\NotSubject',
221222
]
222223
]
@@ -259,11 +260,43 @@ public function getDependencyInfoDataCaseGetUrlDataProvider()
259260
[
260261
[
261262
'modules' => ['Magento\Cms'],
262-
'type' => \Magento\TestFramework\Dependency\RuleInterface::TYPE_HARD,
263+
'type' => RuleInterface::TYPE_HARD,
263264
'source' => 'getUrl("cms/index/index"',
264265
]
265266
]
266267
],
268+
'getUrl from API of same module' => [
269+
'Magento\Catalog\SomeClass',
270+
'$this->getUrl("rest/V1/products/3")',
271+
[]
272+
],
273+
'getUrl from API of different module' => [
274+
'Magento\Backend\SomeClass',
275+
'$this->getUrl("rest/V1/products/43/options")',
276+
[
277+
[
278+
'modules' => ['Magento\Catalog'],
279+
'type' => RuleInterface::TYPE_HARD,
280+
'source' => 'getUrl("rest/V1/products/43/options"'
281+
]
282+
],
283+
],
284+
//Skip processing routeid wildcards
285+
'getUrl from routeid wildcard in controller' => [
286+
'Magento\Catalog\Controller\ControllerName\SomeClass',
287+
'$this->getUrl("*/Invalid/*")',
288+
[]
289+
],
290+
'getUrl from wildcard url within ignored Block class' => [
291+
'Magento\Cms\Block\SomeClass',
292+
'$this->getUrl("Catalog/*/View")',
293+
[]
294+
],
295+
'getUrl from wildcard url for ControllerName' => [
296+
'Magento\Catalog\Controller\Category\IGNORE',
297+
'$this->getUrl("Catalog/*/View")',
298+
[]
299+
],
267300
];
268301
}
269302

@@ -297,6 +330,11 @@ public function getDependencyInfoDataCaseGetUrlExceptionDataProvider()
297330
'$this->getUrl("someModule")',
298331
new LocalizedException(__('Invalid URL path: %1', 'somemodule/index/index')),
299332
],
333+
'getUrl from unknown wildcard path' => [
334+
'Magento\Catalog\Controller\Product\View',
335+
'$this->getUrl("Catalog/*/INVALID")',
336+
new LocalizedException(__('Invalid URL path: %1', 'catalog/product/invalid')),
337+
],
300338
];
301339
}
302340

@@ -332,7 +370,7 @@ public function getDefaultModelDependencyDataProvider()
332370
[
333371
[
334372
'modules' => ['Magento\SomeModule'],
335-
'type' => \Magento\TestFramework\Dependency\RuleInterface::TYPE_HARD,
373+
'type' => RuleInterface::TYPE_HARD,
336374
'source' => 'getBlock(\'block.name\')',
337375
]
338376
],
@@ -363,4 +401,29 @@ private function getModuleFromClass(string $class): string
363401
$moduleNameLength = strpos($class, '\\', strpos($class, '\\') + 1);
364402
return substr($class, 0, $moduleNameLength);
365403
}
404+
405+
/**
406+
* Returns an example list of services that would be parsed via the configReader
407+
*/
408+
private function makeWebApiConfigReaderMock()
409+
{
410+
$services = [ 'routes' => [
411+
'/V1/products/:sku' => [
412+
'GET' => ['service' => [
413+
'class' => 'Magento\Catalog\Api\ProductRepositoryInterface',
414+
'method' => 'get'
415+
] ],
416+
'PUT' => ['service' => [
417+
'class' => 'Magento\Catalog\Api\ProductRepositoryInterface',
418+
'method' => 'save'
419+
] ],
420+
],
421+
'V1/products/:sku/options' => ['GET' => ['service' => [
422+
'class' => 'Magento\Catalog\Api\ProductCustomOptionRepositoryInterface',
423+
'method' => 'getList'
424+
] ] ]
425+
] ];
426+
427+
return $this->createConfiguredMock(Filesystem::class, [ 'read' => $services ]);
428+
}
366429
}

‎dev/tests/static/testsuite/Magento/Test/Integrity/Dependency/WebapiFileResolver.php

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88

99
use Magento\Framework\Component\ComponentRegistrar;
1010

11+
/**
12+
* Collects all webapi.xml files
13+
*/
1114
class WebapiFileResolver implements \Magento\Framework\Config\FileResolverInterface
1215
{
1316
/**
@@ -18,7 +21,7 @@ class WebapiFileResolver implements \Magento\Framework\Config\FileResolverInterf
1821
/**
1922
* @var string[]
2023
*/
21-
private $webapixml_paths;
24+
private $webapiXmlPaths;
2225

2326
public function __construct(ComponentRegistrar $componentRegistrar)
2427
{
@@ -30,17 +33,17 @@ public function __construct(ComponentRegistrar $componentRegistrar)
3033
*/
3134
public function get($filename, $scope)
3235
{
33-
if (!$this->webapixml_paths) {
36+
if (!$this->webapiXmlPaths) {
3437
$paths = $this->componentRegistrar->getPaths(ComponentRegistrar::MODULE);
35-
$webapixml_paths = [];
38+
$webapiXmlPaths = [];
3639
foreach ($paths as $path) {
3740
$path = $path . '/etc/webapi.xml';
3841
if (file_exists($path)) {
39-
$webapixml_paths[$path] = file_get_contents($path);
42+
$webapiXmlPaths[$path] = file_get_contents($path);
4043
}
4144
}
42-
$this->webapixml_paths = $webapixml_paths;
45+
$this->webapiXmlPaths = $webapiXmlPaths;
4346
}
44-
return $this->webapixml_paths;
47+
return $this->webapiXmlPaths;
4548
}
4649
}

0 commit comments

Comments
 (0)