Merge pull request #13712 from nextcloud/bugfix/noid/do-not-load-all-routes

Only load routes of the app which is requested

lib/private/Route/Router.php

@@ -73,7 +73,7 @@ class Router implements IRouter {
 		$this->logger = $logger;
 		$baseUrl = \OC::$WEBROOT;
 		if (!(\OC::$server->getConfig()->getSystemValue('htaccess.IgnoreFrontController', false) === true || getenv('front_controller_active') === 'true')) {
-			$baseUrl = \OC::$server->getURLGenerator()->linkTo('', 'index.php');
+			$baseUrl .= '/index.php';
 		}
 		if (!\OC::$CLI && isset($_SERVER['REQUEST_METHOD'])) {
 			$method = $_SERVER['REQUEST_METHOD'];
@@ -346,13 +346,27 @@ class Router implements IRouter {
 	public function generate($name,
 							 $parameters = [],
 							 $absolute = false) {
+		$referenceType = UrlGenerator::ABSOLUTE_URL;
+		if ($absolute === false) {
+			$referenceType = UrlGenerator::ABSOLUTE_PATH;
+		}
+		$name = $this->fixLegacyRootName($name);
+		if (strpos($name, '.') !== false) {
+			list($appName, $other) = explode('.', $name, 3);
+			// OCS routes are prefixed with "ocs."
+			if ($appName === 'ocs') {
+				$appName = $other;
+			}
+			$this->loadRoutes($appName);
+			try {
+				return $this->getGenerator()->generate($name, $parameters, $referenceType);
+			} catch (RouteNotFoundException $e) {
+			}
+		}
+
+		// Fallback load all routes
 		$this->loadRoutes();
 		try {
-			$referenceType = UrlGenerator::ABSOLUTE_URL;
-			if ($absolute === false) {
-				$referenceType = UrlGenerator::ABSOLUTE_PATH;
-			}
-			$name = $this->fixLegacyRootName($name);
 			return $this->getGenerator()->generate($name, $parameters, $referenceType);
 		} catch (RouteNotFoundException $e) {
 			$this->logger->logException($e, ['level' => ILogger::INFO]);

lib/private/Server.php

@@ -644,16 +644,7 @@ class Server extends ServerContainer implements IServerContainer {
 		/** @deprecated 19.0.0 */
 		$this->registerDeprecatedAlias('L10NFactory', IFactory::class);
 
-		$this->registerService(IURLGenerator::class, function (Server $c) {
-			$config = $c->getConfig();
-			$cacheFactory = $c->getMemCacheFactory();
-			$request = $c->getRequest();
-			return new \OC\URLGenerator(
-				$config,
-				$cacheFactory,
-				$request
-			);
-		});
+		$this->registerAlias(IURLGenerator::class, URLGenerator::class);
 		/** @deprecated 19.0.0 */
 		$this->registerDeprecatedAlias('URLGenerator', IURLGenerator::class);
 

lib/private/URLGenerator.php

@@ -44,6 +44,7 @@ use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IRequest;
 use OCP\IURLGenerator;
+use OCP\Route\IRouter;
 use RuntimeException;
 
 /**
@@ -56,18 +57,17 @@ class URLGenerator implements IURLGenerator {
 	private $cacheFactory;
 	/** @var IRequest */
 	private $request;
+	/** @var IRouter*/
+	private $router;
 
-	/**
-	 * @param IConfig $config
-	 * @param ICacheFactory $cacheFactory
-	 * @param IRequest $request
-	 */
 	public function __construct(IConfig $config,
 								ICacheFactory $cacheFactory,
-								IRequest $request) {
+								IRequest $request,
+								IRouter $router) {
 		$this->config = $config;
 		$this->cacheFactory = $cacheFactory;
 		$this->request = $request;
+		$this->router = $router;
 	}
 
 	/**
@@ -80,8 +80,7 @@ class URLGenerator implements IURLGenerator {
 	 * Returns a url to the given route.
 	 */
 	public function linkToRoute(string $routeName, array $arguments = []): string {
-		// TODO: mock router
-		return \OC::$server->getRouter()->generate($routeName, $arguments);
+		return $this->router->generate($routeName, $arguments);
 	}
 
 	/**
@@ -97,7 +96,7 @@ class URLGenerator implements IURLGenerator {
 	}
 
 	public function linkToOCSRouteAbsolute(string $routeName, array $arguments = []): string {
-		$route = \OC::$server->getRouter()->generate('ocs.'.$routeName, $arguments, false);
+		$route = $this->router->generate('ocs.'.$routeName, $arguments, false);
 
 		$indexPhpPos = strpos($route, '/index.php/');
 		if ($indexPhpPos !== false) {

tests/lib/Route/RouterTest.php

@@ -0,0 +1,52 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2019 Morris Jobke <hey@morrisjobke.de>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace Test\Route;
+
+use OC\Route\Router;
+use OCP\ILogger;
+use Test\TestCase;
+
+/**
+ * Class RouterTest
+ *
+ * @package Test\Route
+ */
+class RouterTest extends TestCase {
+	public function testGenerateConsecutively(): void {
+		/** @var ILogger $logger */
+		$logger = $this->createMock(ILogger::class);
+		$router = new Router($logger);
+
+		$this->assertEquals('/index.php/apps/files/', $router->generate('files.view.index'));
+
+		// the OCS route is the prefixed one for the AppFramework - see /ocs/v1.php for routing details
+		$this->assertEquals('/index.php/ocsapp/apps/dav/api/v1/direct', $router->generate('ocs.dav.direct.getUrl'));
+
+		// special route name - should load all apps and then find the route
+		$this->assertEquals('/index.php/apps/files/ajax/list.php', $router->generate('files_ajax_list'));
+
+		// test caching
+		$this->assertEquals('/index.php/apps/files/', $router->generate('files.view.index'));
+	}
+}

tests/lib/UrlGeneratorTest.php

@@ -12,9 +12,12 @@ use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IRequest;
 use OCP\IURLGenerator;
+use OCP\Route\IRouter;
 
 /**
  * Class UrlGeneratorTest
+ *
+ * @package Test
  */
 class UrlGeneratorTest extends \Test\TestCase {
 
@@ -24,6 +27,8 @@ class UrlGeneratorTest extends \Test\TestCase {
 	private $cacheFactory;
 	/** @var \PHPUnit\Framework\MockObject\MockObject|IRequest */
 	private $request;
+	/** @var \PHPUnit\Framework\MockObject\MockObject|IRouter */
+	private $router;
 	/** @var IURLGenerator */
 	private $urlGenerator;
 	/** @var string */
@@ -34,10 +39,12 @@ class UrlGeneratorTest extends \Test\TestCase {
 		$this->config = $this->createMock(IConfig::class);
 		$this->cacheFactory = $this->createMock(ICacheFactory::class);
 		$this->request = $this->createMock(IRequest::class);
+		$this->router = $this->createMock(IRouter::class);
 		$this->urlGenerator = new \OC\URLGenerator(
 			$this->config,
 			$this->cacheFactory,
-			$this->request
+			$this->request,
+			$this->router
 		);
 		$this->originalWebRoot = \OC::$WEBROOT;
 	}
@@ -84,14 +91,23 @@ class UrlGeneratorTest extends \Test\TestCase {
 	public function testLinkToRouteAbsolute($route, $expected) {
 		$this->mockBaseUrl();
 		\OC::$WEBROOT = '/nextcloud';
+		$this->router->expects($this->once())
+			->method('generate')
+			->willReturnCallback(function ($routeName, $parameters) {
+				if ($routeName === 'core.Preview.getPreview') {
+					return '/index.php/core/preview.png';
+				} elseif ($routeName === 'cloud_federation_api.requesthandlercontroller.addShare') {
+					return '/index.php/ocm/shares';
+				}
+			});
 		$result = $this->urlGenerator->linkToRouteAbsolute($route);
 		$this->assertEquals($expected, $result);
 	}
 
 	public function provideRoutes() {
 		return [
-			['files_ajax_list', 'http://localhost/nextcloud/index.php/apps/files/ajax/list.php'],
 			['core.Preview.getPreview', 'http://localhost/nextcloud/index.php/core/preview.png'],
+			['cloud_federation_api.requesthandlercontroller.addShare', 'http://localhost/nextcloud/index.php/ocm/shares'],
 		];
 	}
 
@@ -169,6 +185,15 @@ class UrlGeneratorTest extends \Test\TestCase {
 	public function testLinkToOCSRouteAbsolute(string $route, string $expected) {
 		$this->mockBaseUrl();
 		\OC::$WEBROOT = '/nextcloud';
+		$this->router->expects($this->once())
+			->method('generate')
+			->willReturnCallback(function ($routeName, $parameters) {
+				if ($routeName === 'ocs.core.OCS.getCapabilities') {
+					return '/index.php/ocsapp/cloud/capabilities';
+				} elseif ($routeName === 'ocs.core.WhatsNew.dismiss') {
+					return '/index.php/ocsapp/core/whatsnew';
+				}
+			});
 		$result = $this->urlGenerator->linkToOCSRouteAbsolute($route);
 		$this->assertEquals($expected, $result);
 	}