MC-5806: Detection of URL dependencies does not work for the Dependency static test

fascinosum · fascinosum · commit fe8f197cc080 · 2019-05-24T17:50:21.000-05:00
diff --git a/app/code/Magento/AdminNotification/composer.json b/app/code/Magento/AdminNotification/composer.json
@@ -11,7 +11,8 @@
         "magento/module-backend": "*",
         "magento/module-media-storage": "*",
         "magento/module-store": "*",
-        "magento/module-ui": "*"
+        "magento/module-ui": "*",
+        "magento/module-config": "*"
     },
     "type": "magento2-module",
     "license": [
diff --git a/app/code/Magento/Catalog/Block/Adminhtml/Category/Tree.php b/app/code/Magento/Catalog/Block/Adminhtml/Category/Tree.php
@@ -253,7 +253,7 @@ public function getLoadTreeUrl($expanded = null)
      */
     public function getNodesUrl()
     {
-        return $this->getUrl('catalog/category/jsonTree');
+        return $this->getUrl('catalog/category/tree');
     }
 
     /**
diff --git a/app/code/Magento/Customer/Block/Account/Dashboard.php b/app/code/Magento/Customer/Block/Account/Dashboard.php
@@ -114,10 +114,13 @@ public function getAddressEditUrl($address)
      * Retrieve the Url for customer orders.
      *
      * @return string
+     * @deprecated Action does not exist
      */
     public function getOrdersUrl()
     {
-        return $this->_urlBuilder->getUrl('customer/order/index', ['_secure' => true]);
+        //phpcs:ignore Magento2.Functions.DiscouragedFunction
+        trigger_error('Method is deprecated', E_USER_DEPRECATED);
+        return '';
     }
 
     /**
@@ -137,7 +140,7 @@ public function getReviewsUrl()
      */
     public function getWishlistUrl()
     {
-        return $this->_urlBuilder->getUrl('customer/wishlist/index', ['_secure' => true]);
+        return $this->_urlBuilder->getUrl('wishlist/index', ['_secure' => true]);
     }
 
     /**
diff --git a/app/code/Magento/Directory/Block/Data.php b/app/code/Magento/Directory/Block/Data.php
@@ -75,7 +75,7 @@ public function __construct(
      */
     public function getLoadrRegionUrl()
     {
-        return $this->getUrl('directory/json/childRegion');
+        return $this->getUrl('directory/json/countryRegion');
     }
 
     /**
diff --git a/app/code/Magento/Paypal/Model/Payflow/Service/Request/SecureToken.php b/app/code/Magento/Paypal/Model/Payflow/Service/Request/SecureToken.php
@@ -69,7 +69,7 @@ public function requestToken(Quote $quote)
         $request->setSecuretokenid($this->mathRandom->getUniqueHash());
         $request->setReturnurl($this->url->getUrl('paypal/transparent/response'));
         $request->setErrorurl($this->url->getUrl('paypal/transparent/response'));
-        $request->setCancelurl($this->url->getUrl('paypal/transparent/cancel'));
+        $request->setCancelurl('');
         $request->setDisablereceipt('TRUE');
         $request->setSilenttran('TRUE');
 
diff --git a/app/code/Magento/Review/Block/Adminhtml/Rss.php b/app/code/Magento/Review/Block/Adminhtml/Rss.php
@@ -45,10 +45,9 @@ public function __construct(
      */
     public function getRssData()
     {
-        $newUrl = $this->getUrl('rss/catalog/review', ['_secure' => true, '_nosecret' => true]);
         $title = __('Pending product review(s)');
 
-        $data = ['title' => $title, 'description' => $title, 'link' => $newUrl, 'charset' => 'UTF-8'];
+        $data = ['title' => $title, 'description' => $title, 'link' => '', 'charset' => 'UTF-8'];
 
         foreach ($this->rssModel->getProductCollection() as $item) {
             if ($item->getStoreId()) {
diff --git a/app/code/Magento/Sales/Block/Order/History.php b/app/code/Magento/Sales/Block/Order/History.php
@@ -149,10 +149,13 @@ public function getViewUrl($order)
     /**
      * @param object $order
      * @return string
+     * @deprecated Action does not exist
      */
     public function getTrackUrl($order)
     {
-        return $this->getUrl('sales/order/track', ['order_id' => $order->getId()]);
+        //phpcs:ignore Magento2.Functions.DiscouragedFunction
+        trigger_error('Method is deprecated', E_USER_DEPRECATED);
+        return '';
     }
 
     /**
diff --git a/app/code/Magento/Sales/Block/Order/Recent.php b/app/code/Magento/Sales/Block/Order/Recent.php
@@ -116,10 +116,13 @@ public function getViewUrl($order)
     /**
      * @param object $order
      * @return string
+     * @deprecated Action does not exist
      */
     public function getTrackUrl($order)
     {
-        return $this->getUrl('sales/order/track', ['order_id' => $order->getId()]);
+        //phpcs:ignore Magento2.Functions.DiscouragedFunction
+        trigger_error('Method is deprecated', E_USER_DEPRECATED);
+        return '';
     }
 
     /**
diff --git a/app/code/Magento/Store/composer.json b/app/code/Magento/Store/composer.json
@@ -11,7 +11,8 @@
         "magento/module-config": "*",
         "magento/module-directory": "*",
         "magento/module-media-storage": "*",
-        "magento/module-ui": "*"
+        "magento/module-ui": "*",
+        "magento/module-backend": "*"
     },
     "suggest": {
         "magento/module-deploy": "*"
diff --git a/app/code/Magento/Tinymce3/composer.json b/app/code/Magento/Tinymce3/composer.json
@@ -7,7 +7,8 @@
         "magento/module-backend": "*",
         "magento/module-ui": "*",
         "magento/module-variable": "*",
-        "magento/module-widget": "*"
+        "magento/module-widget": "*",
+        "magento/module-cms": "*"
 
     },
     "suggest": {
diff --git a/app/code/Magento/Ui/composer.json b/app/code/Magento/Ui/composer.json
@@ -11,7 +11,8 @@
         "magento/module-backend": "*",
         "magento/module-eav": "*",
         "magento/module-store": "*",
-        "magento/module-user": "*"
+        "magento/module-user": "*",
+        "magento/module-cms": "*"
     },
     "suggest": {
         "magento/module-config": "*"
diff --git a/app/code/Magento/Variable/composer.json b/app/code/Magento/Variable/composer.json
@@ -9,7 +9,8 @@
         "magento/framework": "*",
         "magento/module-backend": "*",
         "magento/module-store": "*",
-        "magento/module-config": "*"
+        "magento/module-config": "*",
+        "magento/module-ui": "*"
     },
     "type": "magento2-module",
     "license": [
diff --git a/dev/tests/static/framework/Magento/TestFramework/Dependency/PhpRule.php b/dev/tests/static/framework/Magento/TestFramework/Dependency/PhpRule.php
@@ -8,7 +8,12 @@
 namespace Magento\TestFramework\Dependency;
 
 use Magento\Framework\App\Utility\Files;
+use Magento\Framework\UrlInterface;
+use Magento\TestFramework\Dependency\Route\RouteMapper;
 
+/**
+ * Rule to check the dependencies between modules based on references, getUrl and layout blocks
+ */
 class PhpRule implements RuleInterface
 {
     /**
@@ -34,7 +39,7 @@ class PhpRule implements RuleInterface
      *
      * @var array
      */
-    protected $_mapRouters = [];
+    private $_mapRouters = [];
 
     /**
      * List of layout blocks
@@ -59,18 +64,23 @@ class PhpRule implements RuleInterface
     ];
 
     /**
-     * Constructor
-     *
+     * @var RouteMapper
+     */
+    private $routeMapper;
+
+    /**
      * @param array $mapRouters
      * @param array $mapLayoutBlocks
      * @param array $pluginMap
+     * @throws \Exception
      */
     public function __construct(array $mapRouters, array $mapLayoutBlocks, array $pluginMap = [])
     {
         $this->_mapRouters = $mapRouters;
         $this->_mapLayoutBlocks = $mapLayoutBlocks;
         $this->_namespaces = implode('|', \Magento\Framework\App\Utility\Files::init()->getNamespaces());
         $this->pluginMap = $pluginMap ?: null;
+        $this->routeMapper = new RouteMapper();
     }
 
     /**
@@ -81,6 +91,7 @@ public function __construct(array $mapRouters, array $mapLayoutBlocks, array $pl
      * @param string $file
      * @param string $contents
      * @return array
+     * @throws \Exception
      */
     public function getDependencyInfo($currentModule, $fileType, $file, &$contents)
     {
@@ -95,7 +106,7 @@ public function getDependencyInfo($currentModule, $fileType, $file, &$contents)
         );
         $dependenciesInfo = $this->considerCaseDependencies(
             $dependenciesInfo,
-            $this->_caseGetUrl($currentModule, $contents)
+            $this->_caseGetUrl($currentModule, $contents, $file)
         );
         $dependenciesInfo = $this->considerCaseDependencies(
             $dependenciesInfo,
@@ -111,6 +122,7 @@ public function getDependencyInfo($currentModule, $fileType, $file, &$contents)
      * @param string $file
      * @param string $contents
      * @return array
+     * @throws \Exception
      */
     private function caseClassesAndIdentifiers($currentModule, $file, &$contents)
     {
@@ -184,12 +196,14 @@ private function loadDiFiles()
      * Generate an array of plugin info
      *
      * @return array
+     * @throws \Exception
      */
     private function loadPluginMap()
     {
         if (!$this->pluginMap) {
             foreach ($this->loadDiFiles() as $filepath) {
                 $dom = new \DOMDocument();
+                // phpcs:ignore Magento2.Functions.DiscouragedFunction
                 $dom->loadXML(file_get_contents($filepath));
                 $typeNodes = $dom->getElementsByTagName('type');
                 /** @var \DOMElement $type */
@@ -214,6 +228,7 @@ private function loadPluginMap()
      * @param string $dependent
      * @param string $dependency
      * @return bool
+     * @throws \Exception
      */
     private function isPluginDependency($dependent, $dependency)
     {
@@ -239,29 +254,30 @@ private function isPluginDependency($dependent, $dependency)
      * @param $currentModule
      * @param $contents
      * @return array
+     * @throws \Exception
      */
     protected function _caseGetUrl($currentModule, &$contents)
     {
-        $pattern = '/[\->:]+(?<source>getUrl\([\'"](?<router>[\w\/*]+)[\'"])/';
+        $pattern = '#(\->|:)(?<source>getUrl\(([\'"])(?<route_id>[a-z0-9\-_]{3,})/'
+            .'(?<controller_name>[a-z0-9\-_]+)(/(?<action_name>[a-z0-9\-_]+))?\3)#i';
 
         $dependencies = [];
         if (!preg_match_all($pattern, $contents, $matches, PREG_SET_ORDER)) {
             return $dependencies;
         }
 
         foreach ($matches as $item) {
-            $router = str_replace('/', '\\', $item['router']);
-            if (isset($this->_mapRouters[$router])) {
-                $modules = $this->_mapRouters[$router];
-                if (!in_array($currentModule, $modules)) {
-                    foreach ($modules as $module) {
-                        $dependencies[] = [
-                            'module' => $module,
-                            'type' => RuleInterface::TYPE_HARD,
-                            'source' => $item['source'],
-                        ];
-                    }
-                }
+            $modules = $this->routeMapper->getDependencyByRoutePath(
+                $item['route_id'],
+                $item['controller_name'],
+                $item['action_name'] ?? UrlInterface::DEFAULT_ACTION_NAME
+            );
+            if (!in_array($currentModule, $modules)) {
+                $dependencies[] = [
+                    'module' => implode(" || ", $modules),
+                    'type' => RuleInterface::TYPE_HARD,
+                    'source' => $item['source'],
+                ];
             }
         }
         return $dependencies;
diff --git a/dev/tests/static/framework/Magento/TestFramework/Dependency/Route/RouteMapper.php b/dev/tests/static/framework/Magento/TestFramework/Dependency/Route/RouteMapper.php
diff --git a/dev/tests/static/testsuite/Magento/Test/Integrity/DependencyTest.php b/dev/tests/static/testsuite/Magento/Test/Integrity/DependencyTest.php
diff --git a/dev/tests/static/testsuite/Magento/Test/Integrity/_files/dependency_test/undetected_dependencies_ce.php b/dev/tests/static/testsuite/Magento/Test/Integrity/_files/dependency_test/undetected_dependencies_ce.php

Original file line number	Diff line number	Diff line change
`@@ -253,7 +253,7 @@ public function getLoadTreeUrl($expanded = null)`
`253`	`253`	`*/`
`254`	`254`	`public function getNodesUrl()`
`255`	`255`	`{`
`256`		`- return $this->getUrl('catalog/category/jsonTree');`
	`256`	`+ return $this->getUrl('catalog/category/tree');`
`257`	`257`	`}`
`258`	`258`
`259`	`259`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -114,10 +114,13 @@ public function getAddressEditUrl($address)`
`114`	`114`	`* Retrieve the Url for customer orders.`
`115`	`115`	`*`
`116`	`116`	`* @return string`
	`117`	`+ * @deprecated Action does not exist`
`117`	`118`	`*/`
`118`	`119`	`public function getOrdersUrl()`
`119`	`120`	`{`
`120`		`- return $this->_urlBuilder->getUrl('customer/order/index', ['_secure' => true]);`
	`121`	`+ //phpcs:ignore Magento2.Functions.DiscouragedFunction`
	`122`	`+ trigger_error('Method is deprecated', E_USER_DEPRECATED);`
	`123`	`+ return '';`
`121`	`124`	`}`
`122`	`125`
`123`	`126`	`/**`
`@@ -137,7 +140,7 @@ public function getReviewsUrl()`
`137`	`140`	`*/`
`138`	`141`	`public function getWishlistUrl()`
`139`	`142`	`{`
`140`		`- return $this->_urlBuilder->getUrl('customer/wishlist/index', ['_secure' => true]);`
	`143`	`+ return $this->_urlBuilder->getUrl('wishlist/index', ['_secure' => true]);`
`141`	`144`	`}`
`142`	`145`
`143`	`146`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -75,7 +75,7 @@ public function __construct(`
`75`	`75`	`*/`
`76`	`76`	`public function getLoadrRegionUrl()`
`77`	`77`	`{`
`78`		`- return $this->getUrl('directory/json/childRegion');`
	`78`	`+ return $this->getUrl('directory/json/countryRegion');`
`79`	`79`	`}`
`80`	`80`
`81`	`81`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -149,10 +149,13 @@ public function getViewUrl($order)`
`149`	`149`	`/**`
`150`	`150`	`* @param object $order`
`151`	`151`	`* @return string`
	`152`	`+ * @deprecated Action does not exist`
`152`	`153`	`*/`
`153`	`154`	`public function getTrackUrl($order)`
`154`	`155`	`{`
`155`		`- return $this->getUrl('sales/order/track', ['order_id' => $order->getId()]);`
	`156`	`+ //phpcs:ignore Magento2.Functions.DiscouragedFunction`
	`157`	`+ trigger_error('Method is deprecated', E_USER_DEPRECATED);`
	`158`	`+ return '';`
`156`	`159`	`}`
`157`	`160`
`158`	`161`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -116,10 +116,13 @@ public function getViewUrl($order)`
`116`	`116`	`/**`
`117`	`117`	`* @param object $order`
`118`	`118`	`* @return string`
	`119`	`+ * @deprecated Action does not exist`
`119`	`120`	`*/`
`120`	`121`	`public function getTrackUrl($order)`
`121`	`122`	`{`
`122`		`- return $this->getUrl('sales/order/track', ['order_id' => $order->getId()]);`
	`123`	`+ //phpcs:ignore Magento2.Functions.DiscouragedFunction`
	`124`	`+ trigger_error('Method is deprecated', E_USER_DEPRECATED);`
	`125`	`+ return '';`
`123`	`126`	`}`
`124`	`127`
`125`	`128`	`/**`