From 250467e842a0856a84e9962ed6ef476a2e3ccfef Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 14 Apr 2020 17:53:15 +0200 Subject: [PATCH] Extend tests for root url Signed-off-by: Joas Schilling --- .../AppFramework/Routing/RouteConfig.php | 4 +- .../lib/AppFramework/Routing/RoutingTest.php | 137 +++++++----------- 2 files changed, 52 insertions(+), 89 deletions(-) diff --git a/lib/private/AppFramework/Routing/RouteConfig.php b/lib/private/AppFramework/Routing/RouteConfig.php index af745a35fcc..2f2d51f6e1a 100644 --- a/lib/private/AppFramework/Routing/RouteConfig.php +++ b/lib/private/AppFramework/Routing/RouteConfig.php @@ -55,7 +55,7 @@ class RouteConfig { /** @var string[] */ private $controllerNameCache = []; - public const ROOT_URL_APPS = [ + protected $rootUrlApps = [ 'cloud_federation_api', 'core', 'files_sharing', @@ -126,7 +126,7 @@ class RouteConfig { $postfix = $route['postfix'] ?? ''; $defaultRoot = $this->appName === 'core' ? '' : '/apps/' . $this->appName; $root = $route['root'] ?? $defaultRoot; - if ($routeNamePrefix === '' && !\in_array($this->appName, self::ROOT_URL_APPS, true)) { + if ($routeNamePrefix === '' && !\in_array($this->appName, $this->rootUrlApps, true)) { // Only allow root URLS for some apps $root = $defaultRoot; } 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 - - * */ -- 2.39.5