Merge pull request #20114 from nextcloud/techdebt/noid/allow-some-apps-to-have-root-urls

Allow some apps to have root URLs in their own routing file

2 files changed, 77 insertions, 79 deletions

diff --git a/lib/private/AppFramework/Routing/RouteConfig.php b/lib/private/AppFramework/Routing/RouteConfig.php
index eb9991fbe69..2f2d51f6e1a 100644
--- a/lib/private/AppFramework/Routing/RouteConfig.php
+++ b/lib/private/AppFramework/Routing/RouteConfig.php
@@ -33,7 +33,6 @@ declare(strict_types=1);
 namespace OC\AppFramework\Routing;
 
 use OC\AppFramework\DependencyInjection\DIContainer;
-use OCP\AppFramework\App;
 use OCP\Route\IRouter;
 
 /**
@@ -56,6 +55,14 @@ class RouteConfig {
 	/** @var string[] */
 	private $controllerNameCache = [];
 
+	protected $rootUrlApps = [
+		'cloud_federation_api',
+		'core',
+		'files_sharing',
+		'files',
+		'spreed',
+	];
+
 	/**
 	 * @param \OC\AppFramework\DependencyInjection\DIContainer $container
 	 * @param \OCP\Route\IRouter $router
@@ -98,42 +105,7 @@ class RouteConfig {
 	private function processOCS(array $routes): void {
 		$ocsRoutes = $routes['ocs'] ?? [];
 		foreach ($ocsRoutes as $ocsRoute) {
-			$name = $ocsRoute['name'];
-			$postfix = $ocsRoute['postfix'] ?? '';
-			$root = $ocsRoute['root'] ?? '/apps/' . $this->appName;
-
-			$url = $root . $ocsRoute['url'];
-			$verb = strtoupper($ocsRoute['verb'] ?? 'GET');
-
-			$split = explode('#', $name, 2);
-			if (count($split) !== 2) {
-				throw new \UnexpectedValueException('Invalid route name');
-			}
-			list($controller, $action) = $split;
-
-			$controllerName = $this->buildControllerName($controller);
-			$actionName = $this->buildActionName($action);
-
-			$routeName = 'ocs.' . $this->appName . '.' . $controller . '.' . $action . $postfix;
-
-			// register the route
-			$handler = new RouteActionHandler($this->container, $controllerName, $actionName);
-
-			$router = $this->router->create($routeName, $url)
-				->method($verb)
-				->action($handler);
-
-			// optionally register requirements for route. This is used to
-			// tell the route parser how url parameters should be matched
-			if (array_key_exists('requirements', $ocsRoute)) {
-				$router->requirements($ocsRoute['requirements']);
-			}
-
-			// optionally register defaults for route. This is used to
-			// tell the route parser how url parameters should be default valued
-			if (array_key_exists('defaults', $ocsRoute)) {
-				$router->defaults($ocsRoute['defaults']);
-			}
+			$this->processRoute($ocsRoute, 'ocs.');
 		}
 	}
 
@@ -145,53 +117,51 @@ class RouteConfig {
 	private function processSimpleRoutes(array $routes): void {
 		$simpleRoutes = $routes['routes'] ?? [];
 		foreach ($simpleRoutes as $simpleRoute) {
-			$name = $simpleRoute['name'];
-			$postfix = $simpleRoute['postfix'] ?? '';
+			$this->processRoute($simpleRoute);
+		}
+	}
 
-			$url = $simpleRoute['url'];
-			$verb = strtoupper($simpleRoute['verb'] ?? 'GET');
+	protected function processRoute(array $route, string $routeNamePrefix = ''): void {
+		$name = $route['name'];
+		$postfix = $route['postfix'] ?? '';
+		$defaultRoot = $this->appName === 'core' ? '' : '/apps/' . $this->appName;
+		$root = $route['root'] ?? $defaultRoot;
+		if ($routeNamePrefix === '' && !\in_array($this->appName, $this->rootUrlApps, true)) {
+			// Only allow root URLS for some apps
+			$root = $defaultRoot;
+		}
 
-			$split = explode('#', $name, 2);
-			if (count($split) !== 2) {
-				throw new \UnexpectedValueException('Invalid route name');
-			}
-			list($controller, $action) = $split;
-
-			$controllerName = $this->buildControllerName($controller);
-			$actionName = $this->buildActionName($action);
-			$appName = $simpleRoute['app'] ?? $this->appName;
-
-			if (isset($simpleRoute['app'])) {
-				// Legacy routes that need to be globally available while they are handled by an app
-				// E.g. '/f/{id}', '/s/{token}', '/call/{token}', …
-				$controllerName = str_replace('controllerController', 'Controller', $controllerName);
-				if ($controllerName === 'PublicpreviewController') {
-					$controllerName = 'PublicPreviewController';
-				} elseif ($controllerName === 'RequesthandlerController') {
-					$controllerName = 'RequestHandlerController';
-				}
-				$controllerName = App::buildAppNamespace($appName) . '\\Controller\\' . $controllerName;
-			}
+		$url = $root . $route['url'];
+		$verb = strtoupper($route['verb'] ?? 'GET');
+
+		$split = explode('#', $name, 2);
+		if (count($split) !== 2) {
+			throw new \UnexpectedValueException('Invalid route name');
+		}
+		list($controller, $action) = $split;
 
-			$routeName = $appName . '.' . $controller . '.' . $action . $postfix;
+		$controllerName = $this->buildControllerName($controller);
+		$actionName = $this->buildActionName($action);
 
-			// register the route
-			$handler = new RouteActionHandler($this->container, $controllerName, $actionName);
-			$router = $this->router->create($routeName, $url)
-							->method($verb)
-							->action($handler);
+		$routeName = $routeNamePrefix . $this->appName . '.' . $controller . '.' . $action . $postfix;
 
-			// optionally register requirements for route. This is used to
-			// tell the route parser how url parameters should be matched
-			if (array_key_exists('requirements', $simpleRoute)) {
-				$router->requirements($simpleRoute['requirements']);
-			}
+		// register the route
+		$handler = new RouteActionHandler($this->container, $controllerName, $actionName);
 
-			// optionally register defaults for route. This is used to
-			// tell the route parser how url parameters should be default valued
-			if (array_key_exists('defaults', $simpleRoute)) {
-				$router->defaults($simpleRoute['defaults']);
-			}
+		$router = $this->router->create($routeName, $url)
+			->method($verb)
+			->action($handler);
+
+		// optionally register requirements for route. This is used to
+		// tell the route parser how url parameters should be matched
+		if (array_key_exists('requirements', $route)) {
+			$router->requirements($route['requirements']);
+		}
+
+		// optionally register defaults for route. This is used to
+		// tell the route parser how url parameters should be default valued
+		if (array_key_exists('defaults', $route)) {
+			$router->defaults($route['defaults']);
 		}
 	}
 
diff --git a/lib/private/Route/Router.php b/lib/private/Route/Router.php
index ccb1578f8d3..075b81224ad 100644
--- a/lib/private/Route/Router.php
+++ b/lib/private/Route/Router.php
@@ -151,7 +151,6 @@ class Router implements IRouter {
 				$this->useCollection($app);
 				$this->requireRouteFile($file, $app);
 				$collection = $this->getCollection($app);
-				$collection->addPrefix('/apps/' . $app);
 				$this->root->addCollection($collection);
 
 				// Also add the OCS collection
@@ -336,6 +335,7 @@ class Router implements IRouter {
 			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);
@@ -343,6 +343,34 @@ class Router implements IRouter {
 		}
 	}
 
+	protected function fixLegacyRootName(string $routeName): string {
+		if ($routeName === 'files.viewcontroller.showFile') {
+			return 'files.View.showFile';
+		}
+		if ($routeName === 'files_sharing.sharecontroller.showShare') {
+			return 'files_sharing.Share.showShare';
+		}
+		if ($routeName === 'files_sharing.sharecontroller.showAuthenticate') {
+			return 'files_sharing.Share.showAuthenticate';
+		}
+		if ($routeName === 'files_sharing.sharecontroller.authenticate') {
+			return 'files_sharing.Share.authenticate';
+		}
+		if ($routeName === 'files_sharing.sharecontroller.downloadShare') {
+			return 'files_sharing.Share.downloadShare';
+		}
+		if ($routeName === 'files_sharing.publicpreview.directLink') {
+			return 'files_sharing.PublicPreview.directLink';
+		}
+		if ($routeName === 'cloud_federation_api.requesthandlercontroller.addShare') {
+			return 'cloud_federation_api.RequestHandler.addShare';
+		}
+		if ($routeName === 'cloud_federation_api.requesthandlercontroller.receiveNotification') {
+			return 'cloud_federation_api.RequestHandler.receiveNotification';
+		}
+		return $routeName;
+	}
+
 	/**
 	 * To isolate the variable scope used inside the $file it is required in it's own method
 	 *