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

1 files changed, 50 insertions, 87 deletions

diff --git a/tests/lib/AppFramework/Routing/RoutingTest.php b/tests/lib/AppFramework/Routing/RoutingTest.php
index 1aef757720b..f9cd181468f 100644
--- a/tests/lib/AppFramework/Routing/RoutingTest.php
+++ b/tests/lib/AppFramework/Routing/RoutingTest.php
@@ -5,6 +5,7 @@ namespace Test\AppFramework\Routing;
 use OC\AppFramework\DependencyInjection\DIContainer;
 use OC\AppFramework\Routing\RouteActionHandler;
 use OC\AppFramework\Routing\RouteConfig;
+use OC\Route\Route;
 use OC\Route\Router;
 use OCP\ILogger;
 use OCP\Route\IRouter;
@@ -16,7 +17,15 @@ class RoutingTest extends \Test\TestCase {
 			['name' => 'folders#open', 'url' => '/folders/{folderId}/open', 'verb' => 'GET']
 		]];
 
-		$this->assertSimpleRoute($routes, 'folders.open', 'GET', '/folders/{folderId}/open', 'FoldersController', 'open');
+		$this->assertSimpleRoute($routes, 'folders.open', 'GET', '/apps/app1/folders/{folderId}/open', 'FoldersController', 'open');
+	}
+
+	public function testSimpleRouteWithUnderScoreNames() {
+		$routes = ['routes' => [
+			['name' => 'admin_folders#open_current', 'url' => '/folders/{folderId}/open', 'verb' => 'delete', 'root' => '']
+		]];
+
+		$this->assertSimpleRoute($routes, 'admin_folders.open_current', 'DELETE', '/folders/{folderId}/open', 'AdminFoldersController', 'openCurrent', [], [], '', true);
 	}
 
 	public function testSimpleOCSRoute() {
@@ -33,7 +42,7 @@ class RoutingTest extends \Test\TestCase {
 			['name' => 'folders#open', 'url' => '/folders/{folderId}/open']
 		]];
 
-		$this->assertSimpleRoute($routes, 'folders.open', 'GET', '/folders/{folderId}/open', 'FoldersController', 'open');
+		$this->assertSimpleRoute($routes, 'folders.open', 'GET', '/apps/app1/folders/{folderId}/open', 'FoldersController', 'open');
 	}
 
 	public function testSimpleOCSRouteWithMissingVerb() {
@@ -50,7 +59,7 @@ class RoutingTest extends \Test\TestCase {
 			['name' => 'folders#open', 'url' => '/folders/{folderId}/open', 'verb' => 'delete']
 		]];
 
-		$this->assertSimpleRoute($routes, 'folders.open', 'DELETE', '/folders/{folderId}/open', 'FoldersController', 'open');
+		$this->assertSimpleRoute($routes, 'folders.open', 'DELETE', '/apps/app1/folders/{folderId}/open', 'FoldersController', 'open');
 	}
 
 	public function testSimpleOCSRouteWithLowercaseVerb() {
@@ -67,7 +76,7 @@ class RoutingTest extends \Test\TestCase {
 			['name' => 'folders#open', 'url' => '/folders/{folderId}/open', 'verb' => 'delete', 'requirements' => ['something']]
 		]];
 
-		$this->assertSimpleRoute($routes, 'folders.open', 'DELETE', '/folders/{folderId}/open', 'FoldersController', 'open', ['something']);
+		$this->assertSimpleRoute($routes, 'folders.open', 'DELETE', '/apps/app1/folders/{folderId}/open', 'FoldersController', 'open', ['something']);
 	}
 
 	public function testSimpleOCSRouteWithRequirements() {
@@ -84,7 +93,7 @@ class RoutingTest extends \Test\TestCase {
 			['name' => 'folders#open', 'url' => '/folders/{folderId}/open', 'verb' => 'delete', [], 'defaults' => ['param' => 'foobar']]
 		]];
 
-		$this->assertSimpleRoute($routes, 'folders.open', 'DELETE', '/folders/{folderId}/open', 'FoldersController', 'open', [], ['param' => 'foobar']);
+		$this->assertSimpleRoute($routes, 'folders.open', 'DELETE', '/apps/app1/folders/{folderId}/open', 'FoldersController', 'open', [], ['param' => 'foobar']);
 	}
 
 
@@ -102,7 +111,7 @@ class RoutingTest extends \Test\TestCase {
 			['name' => 'folders#open', 'url' => '/folders/{folderId}/open', 'verb' => 'delete', 'postfix' => '_something']
 		]];
 
-		$this->assertSimpleRoute($routes, 'folders.open', 'DELETE', '/folders/{folderId}/open', 'FoldersController', 'open', [], [], '_something');
+		$this->assertSimpleRoute($routes, 'folders.open', 'DELETE', '/apps/app1/folders/{folderId}/open', 'FoldersController', 'open', [], [], '_something');
 	}
 
 	public function testSimpleOCSRouteWithPostfix() {
@@ -114,7 +123,7 @@ class RoutingTest extends \Test\TestCase {
 		$this->assertSimpleOCSRoute($routes, 'folders.open', 'DELETE', '/apps/app1/folders/{folderId}/open', 'FoldersController', 'open', [], [], '_something');
 	}
 
-	
+
 	public function testSimpleRouteWithBrokenName() {
 		$this->expectException(\UnexpectedValueException::class);
 
@@ -122,10 +131,10 @@ class RoutingTest extends \Test\TestCase {
 			['name' => 'folders_open', 'url' => '/folders/{folderId}/open', 'verb' => 'delete']
 		]];
 
-		// router mock
-		$router = $this->getMockBuilder('\OC\Route\Router')
-			->setMethods(['create'])
-			->setConstructorArgs([$this->getMockBuilder(ILogger::class)->getMock()])
+		/** @var IRouter|MockObject $router */
+		$router = $this->getMockBuilder(Router::class)
+			->onlyMethods(['create'])
+			->setConstructorArgs([$this->createMock(ILogger::class)])
 			->getMock();
 
 		// load route configuration
@@ -135,7 +144,7 @@ class RoutingTest extends \Test\TestCase {
 		$config->register();
 	}
 
-	
+
 	public function testSimpleOCSRouteWithBrokenName() {
 		$this->expectException(\UnexpectedValueException::class);
 
@@ -143,10 +152,10 @@ class RoutingTest extends \Test\TestCase {
 			['name' => 'folders_open', 'url' => '/folders/{folderId}/open', 'verb' => 'delete']
 		]];
 
-		// router mock
-		$router = $this->getMockBuilder('\OC\Route\Router')
-			->setMethods(['create'])
-			->setConstructorArgs([$this->getMockBuilder(ILogger::class)->getMock()])
+		/** @var IRouter|MockObject $router */
+		$router = $this->getMockBuilder(Router::class)
+			->onlyMethods(['create'])
+			->setConstructorArgs([$this->createMock(ILogger::class)])
 			->getMock();
 
 		// load route configuration
@@ -156,14 +165,6 @@ class RoutingTest extends \Test\TestCase {
 		$config->register();
 	}
 
-	public function testSimpleRouteWithUnderScoreNames() {
-		$routes = ['routes' => [
-			['name' => 'admin_folders#open_current', 'url' => '/folders/{folderId}/open', 'verb' => 'delete']
-		]];
-
-		$this->assertSimpleRoute($routes, 'admin_folders.open_current', 'DELETE', '/folders/{folderId}/open', 'AdminFoldersController', 'openCurrent');
-	}
-
 	public function testSimpleOCSRouteWithUnderScoreNames() {
 		$routes = ['ocs' => [
 			['name' => 'admin_folders#open_current', 'url' => '/folders/{folderId}/open', 'verb' => 'delete']
@@ -202,14 +203,7 @@ class RoutingTest extends \Test\TestCase {
 		$this->assertResource($routes, 'admin_accounts', '/admin/accounts', 'AdminAccountsController', 'id');
 	}
 
-	/**
-	 * @param string $name
-	 * @param string $verb
-	 * @param string $url
-	 * @param string $controllerName
-	 * @param string $actionName
-	 */
-	private function assertSimpleRoute($routes, $name, $verb, $url, $controllerName, $actionName, array $requirements=[], array $defaults=[], $postfix='') {
+	private function assertSimpleRoute($routes, $name, $verb, $url, $controllerName, $actionName, array $requirements = [], array $defaults = [], $postfix = '', $allowRootUrl = false): void {
 		if ($postfix) {
 			$name .= $postfix;
 		}
@@ -218,10 +212,10 @@ class RoutingTest extends \Test\TestCase {
 		$container = new DIContainer('app1');
 		$route = $this->mockRoute($container, $verb, $controllerName, $actionName, $requirements, $defaults);
 
-		// router mock
-		$router = $this->getMockBuilder('\OC\Route\Router')
-			->setMethods(['create'])
-			->setConstructorArgs([$this->getMockBuilder(ILogger::class)->getMock()])
+		/** @var IRouter|MockObject $router */
+		$router = $this->getMockBuilder(Router::class)
+			->onlyMethods(['create'])
+			->setConstructorArgs([$this->createMock(ILogger::class)])
 			->getMock();
 
 		// we expect create to be called once:
@@ -233,6 +227,9 @@ class RoutingTest extends \Test\TestCase {
 
 		// load route configuration
 		$config = new RouteConfig($container, $router, $routes);
+		if ($allowRootUrl) {
+			self::invokePrivate($config, 'rootUrlApps', [['app1']]);
+		}
 
 		$config->register();
 	}
@@ -265,10 +262,10 @@ class RoutingTest extends \Test\TestCase {
 		$container = new DIContainer('app1');
 		$route = $this->mockRoute($container, $verb, $controllerName, $actionName, $requirements, $defaults);
 
-		// router mock
-		$router = $this->getMockBuilder('\OC\Route\Router')
-			->setMethods(['create'])
-			->setConstructorArgs([$this->getMockBuilder(ILogger::class)->getMock()])
+		/** @var IRouter|MockObject $router */
+		$router = $this->getMockBuilder(Router::class)
+			->onlyMethods(['create'])
+			->setConstructorArgs([$this->createMock(ILogger::class)])
 			->getMock();
 
 		// we expect create to be called once:
@@ -294,8 +291,8 @@ class RoutingTest extends \Test\TestCase {
 	private function assertOCSResource($yaml, $resourceName, $url, $controllerName, $paramName): void {
 		/** @var IRouter|MockObject $router */
 		$router = $this->getMockBuilder(Router::class)
-			->setMethods(['create'])
-			->setConstructorArgs([$this->getMockBuilder(ILogger::class)->getMock()])
+			->onlyMethods(['create'])
+			->setConstructorArgs([$this->createMock(ILogger::class)])
 			->getMock();
 
 		// route mocks
@@ -352,10 +349,10 @@ class RoutingTest extends \Test\TestCase {
 	 * @param string $paramName
 	 */
 	private function assertResource($yaml, $resourceName, $url, $controllerName, $paramName) {
-		// router mock
-		$router = $this->getMockBuilder('\OC\Route\Router')
-			->setMethods(['create'])
-			->setConstructorArgs([$this->getMockBuilder(ILogger::class)->getMock()])
+		/** @var IRouter|MockObject $router */
+		$router = $this->getMockBuilder(Router::class)
+			->onlyMethods(['create'])
+			->setConstructorArgs([$this->createMock(ILogger::class)])
 			->getMock();
 
 		// route mocks
@@ -412,7 +409,7 @@ class RoutingTest extends \Test\TestCase {
 	 * @param string $actionName
 	 * @param array $requirements
 	 * @param array $defaults
-	 * @return \PHPUnit_Framework_MockObject_MockObject
+	 * @return MockObject
 	 */
 	private function mockRoute(
 		DIContainer $container,
@@ -422,25 +419,25 @@ class RoutingTest extends \Test\TestCase {
 		array $requirements=[],
 		array $defaults=[]
 	) {
-		$route = $this->getMockBuilder('\OC\Route\Route')
-			->setMethods(['method', 'action', 'requirements', 'defaults'])
+		$route = $this->getMockBuilder(Route::class)
+			->onlyMethods(['method', 'action', 'requirements', 'defaults'])
 			->disableOriginalConstructor()
 			->getMock();
 		$route
-			->expects($this->exactly(1))
+			->expects($this->once())
 			->method('method')
 			->with($this->equalTo($verb))
 			->willReturn($route);
 
 		$route
-			->expects($this->exactly(1))
+			->expects($this->once())
 			->method('action')
 			->with($this->equalTo(new RouteActionHandler($container, $controllerName, $actionName)))
 			->willReturn($route);
 
 		if (count($requirements) > 0) {
 			$route
-				->expects($this->exactly(1))
+				->expects($this->once())
 				->method('requirements')
 				->with($this->equalTo($requirements))
 				->willReturn($route);
@@ -448,7 +445,7 @@ class RoutingTest extends \Test\TestCase {
 
 		if (count($defaults) > 0) {
 			$route
-				->expects($this->exactly(1))
+				->expects($this->once())
 				->method('defaults')
 				->with($this->equalTo($defaults))
 				->willReturn($route);
@@ -457,37 +454,3 @@ class RoutingTest extends \Test\TestCase {
 		return $route;
 	}
 }
-
-/*
-#
-# sample routes.yaml for ownCloud
-#
-# the section simple describes one route
-
-routes:
-		- name: folders#open
-		  url: /folders/{folderId}/open
-		  verb: GET
-		  # controller: name.split()[0]
-		  # action: name.split()[1]
-
-# for a resource following actions will be generated:
-# - index
-# - create
-# - show
-# - update
-# - destroy
-# - new
-resources:
-	accounts:
-		url: /accounts
-
-	folders:
-		url: /accounts/{accountId}/folders
-		# actions can be used to define additional actions on the resource
-		actions:
-			- name: validate
-			  verb: GET
-			  on-collection: false
-
- * */