Merge pull request #13714 from nextcloud/feature/noid/ocs-resources

Allow OCS resources as well

2 files changed, 179 insertions, 54 deletions

diff --git a/lib/private/AppFramework/Routing/RouteConfig.php b/lib/private/AppFramework/Routing/RouteConfig.php
index 70208725f46..0477f4dc209 100644
--- a/lib/private/AppFramework/Routing/RouteConfig.php
+++ b/lib/private/AppFramework/Routing/RouteConfig.php
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  *
@@ -79,47 +80,42 @@ class RouteConfig {
 		 * OCS routes go into a different collection
 		 */
 		$oldCollection = $this->router->getCurrentCollection();
-		$this->router->useCollection($oldCollection.'.ocs');
+		$this->router->useCollection($oldCollection . '.ocs');
 
 		// parse ocs simple routes
 		$this->processOCS($this->routes);
 
+		// parse ocs simple routes
+		$this->processOCSResources($this->routes);
+
 		$this->router->useCollection($oldCollection);
 	}
 
-	private function processOCS(array $routes) {
-		$ocsRoutes = isset($routes['ocs']) ? $routes['ocs'] : [];
+	private function processOCS(array $routes): void {
+		$ocsRoutes = $routes['ocs'] ?? [];
 		foreach ($ocsRoutes as $ocsRoute) {
 			$name = $ocsRoute['name'];
-			$postfix = '';
-
-			if (isset($ocsRoute['postfix'])) {
-				$postfix = $ocsRoute['postfix'];
-			}
-
-			if (isset($ocsRoute['root'])) {
-				$root = $ocsRoute['root'];
-			} else {
-				$root = '/apps/'.$this->appName;
-			}
+			$postfix = $ocsRoute['postfix'] ?? '';
+			$root = $ocsRoute['root'] ?? '/apps/' . $this->appName;
 
 			$url = $root . $ocsRoute['url'];
-			$verb = isset($ocsRoute['verb']) ? strtoupper($ocsRoute['verb']) : 'GET';
+			$verb = strtoupper($ocsRoute['verb'] ?? 'GET');
 
 			$split = explode('#', $name, 2);
-			if (count($split) != 2) {
+			if (count($split) !== 2) {
 				throw new \UnexpectedValueException('Invalid route name');
 			}
-			$controller = $split[0];
-			$action = $split[1];
+			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('ocs.'.$this->appName.'.'.$controller.'.'.$action . $postfix, $url)
+			$router = $this->router->create($routeName, $url)
 				->method($verb)
 				->action($handler);
 
@@ -142,33 +138,29 @@ class RouteConfig {
 	 * @param array $routes
 	 * @throws \UnexpectedValueException
 	 */
-	private function processSimpleRoutes($routes)
-	{
-		$simpleRoutes = isset($routes['routes']) ? $routes['routes'] : array();
+	private function processSimpleRoutes(array $routes): void {
+		$simpleRoutes = $routes['routes'] ?? [];
 		foreach ($simpleRoutes as $simpleRoute) {
 			$name = $simpleRoute['name'];
-			$postfix = '';
-
-			if (isset($simpleRoute['postfix'])) {
-				$postfix = $simpleRoute['postfix'];
-			}
+			$postfix = $simpleRoute['postfix'] ?? '';
 
 			$url = $simpleRoute['url'];
-			$verb = isset($simpleRoute['verb']) ? strtoupper($simpleRoute['verb']) : 'GET';
+			$verb = strtoupper($simpleRoute['verb'] ?? 'GET');
 
 			$split = explode('#', $name, 2);
-			if (count($split) != 2) {
+			if (count($split) !== 2) {
 				throw new \UnexpectedValueException('Invalid route name');
 			}
-			$controller = $split[0];
-			$action = $split[1];
+			list($controller, $action) = $split;
 
 			$controllerName = $this->buildControllerName($controller);
 			$actionName = $this->buildActionName($action);
 
+			$routeName = $this->appName . '.' . $controller . '.' . $action . $postfix;
+
 			// register the route
 			$handler = new RouteActionHandler($this->container, $controllerName, $actionName);
-			$router = $this->router->create($this->appName.'.'.$controller.'.'.$action . $postfix, $url)
+			$router = $this->router->create($routeName, $url)
 							->method($verb)
 							->action($handler);
 
@@ -187,41 +179,90 @@ class RouteConfig {
 	}
 
 	/**
+	 * For a given name and url restful OCS routes are created:
+	 *  - index
+	 *  - show
+	 *  - create
+	 *  - update
+	 *  - destroy
+	 *
+	 * @param array $routes
+	 */
+	private function processOCSResources(array $routes): void {
+		// declaration of all restful actions
+		$actions = [
+			['name' => 'index', 'verb' => 'GET', 'on-collection' => true],
+			['name' => 'show', 'verb' => 'GET'],
+			['name' => 'create', 'verb' => 'POST', 'on-collection' => true],
+			['name' => 'update', 'verb' => 'PUT'],
+			['name' => 'destroy', 'verb' => 'DELETE'],
+		];
+
+		$resources = $routes['ocs-resources'] ?? [];
+		foreach ($resources as $resource => $config) {
+			$root = $config['root'] ?? '/apps/' . $this->appName;
+
+			// the url parameter used as id to the resource
+			foreach($actions as $action) {
+				$url = $root . $config['url'];
+				$method = $action['name'];
+				$verb = strtoupper($action['verb'] ?? 'GET');
+				$collectionAction = $action['on-collection'] ?? false;
+				if (!$collectionAction) {
+					$url .= '/{id}';
+				}
+				if (isset($action['url-postfix'])) {
+					$url .= '/' . $action['url-postfix'];
+				}
+
+				$controller = $resource;
+
+				$controllerName = $this->buildControllerName($controller);
+				$actionName = $this->buildActionName($method);
+
+				$routeName = 'ocs.' . $this->appName . '.' . strtolower($resource) . '.' . strtolower($method);
+
+				$this->router->create($routeName, $url)->method($verb)->action(
+					new RouteActionHandler($this->container, $controllerName, $actionName)
+				);
+			}
+		}
+	}
+
+	/**
 	 * For a given name and url restful routes are created:
 	 *  - index
 	 *  - show
-	 *  - new
 	 *  - create
 	 *  - update
 	 *  - destroy
 	 *
 	 * @param array $routes
 	 */
-	private function processResources($routes)
-	{
+	private function processResources(array $routes): void {
 		// declaration of all restful actions
-		$actions = array(
-			array('name' => 'index', 'verb' => 'GET', 'on-collection' => true),
-			array('name' => 'show', 'verb' => 'GET'),
-			array('name' => 'create', 'verb' => 'POST', 'on-collection' => true),
-			array('name' => 'update', 'verb' => 'PUT'),
-			array('name' => 'destroy', 'verb' => 'DELETE'),
-		);
-
-		$resources = isset($routes['resources']) ? $routes['resources'] : array();
+		$actions = [
+			['name' => 'index', 'verb' => 'GET', 'on-collection' => true],
+			['name' => 'show', 'verb' => 'GET'],
+			['name' => 'create', 'verb' => 'POST', 'on-collection' => true],
+			['name' => 'update', 'verb' => 'PUT'],
+			['name' => 'destroy', 'verb' => 'DELETE'],
+		];
+
+		$resources = $routes['resources'] ?? [];
 		foreach ($resources as $resource => $config) {
 
 			// the url parameter used as id to the resource
 			foreach($actions as $action) {
 				$url = $config['url'];
 				$method = $action['name'];
-				$verb = isset($action['verb']) ? strtoupper($action['verb']) : 'GET';
-				$collectionAction = isset($action['on-collection']) ? $action['on-collection'] : false;
+				$verb = strtoupper($action['verb'] ?? 'GET');
+				$collectionAction = $action['on-collection'] ?? false;
 				if (!$collectionAction) {
-					$url = $url . '/{id}';
+					$url .= '/{id}';
 				}
 				if (isset($action['url-postfix'])) {
-					$url = $url . '/' . $action['url-postfix'];
+					$url .= '/' . $action['url-postfix'];
 				}
 
 				$controller = $resource;
@@ -243,8 +284,7 @@ class RouteConfig {
 	 * @param string $controller
 	 * @return string
 	 */
-	private function buildControllerName($controller)
-	{
+	private function buildControllerName(string $controller): string {
 		if (!isset($this->controllerNameCache[$controller])) {
 			$this->controllerNameCache[$controller] = $this->underScoreToCamelCase(ucfirst($controller)) . 'Controller';
 		}
@@ -256,7 +296,7 @@ class RouteConfig {
 	 * @param string $action
 	 * @return string
 	 */
-	private function buildActionName($action) {
+	private function buildActionName(string $action): string {
 		return $this->underScoreToCamelCase($action);
 	}
 
@@ -265,12 +305,12 @@ class RouteConfig {
 	 * @param string $str
 	 * @return string
 	 */
-	private function underScoreToCamelCase($str) {
-		$pattern = "/_[a-z]?/";
+	private function underScoreToCamelCase(string $str): string {
+		$pattern = '/_[a-z]?/';
 		return preg_replace_callback(
 			$pattern,
 			function ($matches) {
-				return strtoupper(ltrim($matches[0], "_"));
+				return strtoupper(ltrim($matches[0], '_'));
 			},
 			$str);
 	}
diff --git a/tests/lib/AppFramework/Routing/RoutingTest.php b/tests/lib/AppFramework/Routing/RoutingTest.php
index 76533fff014..fccece481ce 100644
--- a/tests/lib/AppFramework/Routing/RoutingTest.php
+++ b/tests/lib/AppFramework/Routing/RoutingTest.php
@@ -6,6 +6,9 @@ use OC\AppFramework\DependencyInjection\DIContainer;
 use OC\AppFramework\Routing\RouteActionHandler;
 use OC\AppFramework\Routing\RouteConfig;
 use OCP\ILogger;
+use OCP\Route\IRouter;
+use PHPUnit\Framework\MockObject\MockObject;
+use OC\Route\Router;
 
 class RoutingTest extends \Test\TestCase
 {
@@ -179,6 +182,27 @@ class RoutingTest extends \Test\TestCase
 		$this->assertSimpleOCSRoute($routes, 'admin_folders.open_current', 'DELETE', '/apps/app1/folders/{folderId}/open', 'AdminFoldersController', 'openCurrent');
 	}
 
+	public function testOCSResource()
+	{
+		$routes = ['ocs-resources' => ['account' => ['url' => '/accounts']]];
+
+		$this->assertOCSResource($routes, 'account', '/apps/app1/accounts', 'AccountController', 'id');
+	}
+
+	public function testOCSResourceWithUnderScoreName()
+	{
+		$routes = ['ocs-resources' => ['admin_accounts' => ['url' => '/admin/accounts']]];
+
+		$this->assertOCSResource($routes, 'admin_accounts', '/apps/app1/admin/accounts', 'AdminAccountsController', 'id');
+	}
+
+	public function testOCSResourceWithRoot()
+	{
+		$routes = ['ocs-resources' => ['admin_accounts' => ['url' => '/admin/accounts', 'root' => '/core/endpoint']]];
+
+		$this->assertOCSResource($routes, 'admin_accounts', '/core/endpoint/admin/accounts', 'AdminAccountsController', 'id');
+	}
+
 	public function testResource()
 	{
 		$routes = array('resources' => array('account' => array('url' => '/accounts')));
@@ -278,6 +302,67 @@ class RoutingTest extends \Test\TestCase
 	}
 
 	/**
+	 * @param array $yaml
+	 * @param string $resourceName
+	 * @param string $url
+	 * @param string $controllerName
+	 * @param string $paramName
+	 */
+	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()])
+			->getMock();
+
+		// route mocks
+		$container = new DIContainer('app1');
+		$indexRoute = $this->mockRoute($container, 'GET', $controllerName, 'index');
+		$showRoute = $this->mockRoute($container, 'GET', $controllerName, 'show');
+		$createRoute = $this->mockRoute($container, 'POST', $controllerName, 'create');
+		$updateRoute = $this->mockRoute($container, 'PUT', $controllerName, 'update');
+		$destroyRoute = $this->mockRoute($container, 'DELETE', $controllerName, 'destroy');
+
+		$urlWithParam = $url . '/{' . $paramName . '}';
+
+		// we expect create to be called once:
+		$router
+			->expects($this->at(0))
+			->method('create')
+			->with($this->equalTo('ocs.app1.' . $resourceName . '.index'), $this->equalTo($url))
+			->willReturn($indexRoute);
+
+		$router
+			->expects($this->at(1))
+			->method('create')
+			->with($this->equalTo('ocs.app1.' . $resourceName . '.show'), $this->equalTo($urlWithParam))
+			->willReturn($showRoute);
+
+		$router
+			->expects($this->at(2))
+			->method('create')
+			->with($this->equalTo('ocs.app1.' . $resourceName . '.create'), $this->equalTo($url))
+			->willReturn($createRoute);
+
+		$router
+			->expects($this->at(3))
+			->method('create')
+			->with($this->equalTo('ocs.app1.' . $resourceName . '.update'), $this->equalTo($urlWithParam))
+			->willReturn($updateRoute);
+
+		$router
+			->expects($this->at(4))
+			->method('create')
+			->with($this->equalTo('ocs.app1.' . $resourceName . '.destroy'), $this->equalTo($urlWithParam))
+			->willReturn($destroyRoute);
+
+		// load route configuration
+		$config = new RouteConfig($container, $router, $yaml);
+
+		$config->register();
+	}
+
+	/**
 	 * @param string $resourceName
 	 * @param string $url
 	 * @param string $controllerName