Move Groups over to OCSController

* Take advantage of the AppFramework
* Fix tests

3 files changed, 138 insertions, 214 deletions

diff --git a/apps/provisioning_api/appinfo/routes.php b/apps/provisioning_api/appinfo/routes.php
index 907e16ebf3c..dbc0321efb2 100644
--- a/apps/provisioning_api/appinfo/routes.php
+++ b/apps/provisioning_api/appinfo/routes.php
@@ -26,13 +26,22 @@
  *
  */
 
-namespace OCA\Provisioning_API\AppInfo;
-
 use OCA\Provisioning_API\Apps;
-use OCA\Provisioning_API\Groups;
 use OCA\Provisioning_API\Users;
 use OCP\API;
 
+$app = new \OCA\Provisioning_API\AppInfo\Application();
+$app->registerRoutes($this, [
+	'ocs' => [
+		// Groups
+		['root' => '/cloud', 'name' => 'Groups#getGroups', 'url' => '/groups', 'verb' => 'GET'],
+		['root' => '/cloud', 'name' => 'Groups#getGroup', 'url' => '/groups/{groupId}', 'verb' => 'GET'],
+		['root' => '/cloud', 'name' => 'Groups#addGroup', 'url' => '/groups', 'verb' => 'POST'],
+		['root' => '/cloud', 'name' => 'Groups#deleteGroup', 'url' => '/groups/{groupId}', 'verb' => 'DELETE'],
+		['root' => '/cloud', 'name' => 'Groups#getSubAdminsOfGroup', 'url' => '/groups/{groupId}/subadmins', 'verb' => 'GET'],
+	],
+]);
+
 // Users
 $users = new Users(
 	\OC::$server->getUserManager(),
@@ -55,18 +64,6 @@ API::register('post', '/cloud/users/{userid}/subadmins', [$users, 'addSubAdmin']
 API::register('delete', '/cloud/users/{userid}/subadmins', [$users, 'removeSubAdmin'], 'provisioning_api', API::ADMIN_AUTH);
 API::register('get', '/cloud/users/{userid}/subadmins', [$users, 'getUserSubAdminGroups'], 'provisioning_api', API::ADMIN_AUTH);
 
-// Groups
-$groups = new Groups(
-	\OC::$server->getGroupManager(),
-	\OC::$server->getUserSession(),
-	\OC::$server->getRequest()
-);
-API::register('get', '/cloud/groups', [$groups, 'getGroups'], 'provisioning_api', API::SUBADMIN_AUTH);
-API::register('post', '/cloud/groups', [$groups, 'addGroup'], 'provisioning_api', API::SUBADMIN_AUTH);
-API::register('get', '/cloud/groups/{groupid}', [$groups, 'getGroup'], 'provisioning_api', API::SUBADMIN_AUTH);
-API::register('delete', '/cloud/groups/{groupid}', [$groups, 'deleteGroup'], 'provisioning_api', API::ADMIN_AUTH);
-API::register('get', '/cloud/groups/{groupid}/subadmins', [$groups, 'getSubAdminsOfGroup'], 'provisioning_api', API::ADMIN_AUTH);
-
 // Apps
 $apps = new Apps(
 	\OC::$server->getAppManager(),
diff --git a/apps/provisioning_api/lib/Groups.php b/apps/provisioning_api/lib/Controller/GroupsController.php
index 18302595ae9..d36d0de8997 100644
--- a/apps/provisioning_api/lib/Groups.php
+++ b/apps/provisioning_api/lib/Controller/GroupsController.php
@@ -23,46 +23,54 @@
  *
  */
 
-namespace OCA\Provisioning_API;
+namespace OCA\Provisioning_API\Controller;
 
+use OCP\AppFramework\Http\DataResponse;
+use OCP\AppFramework\OCS\OCSException;
+use OCP\AppFramework\OCSController;
 use OCP\IGroup;
+use OCP\IGroupManager;
+use OCP\IRequest;
+use OCP\IUserSession;
 use OCP\IUser;
 
-class Groups{
 
-	/** @var \OCP\IGroupManager */
+class GroupsController extends OCSController {
+
+	/** @var IGroupManager */
 	private $groupManager;
 
-	/** @var \OCP\IUserSession */
+	/** @var IUserSession */
 	private $userSession;
 
-	/** @var \OCP\IRequest */
-	private $request;
-
 	/**
-	 * @param \OCP\IGroupManager $groupManager
-	 * @param \OCP\IUserSession $userSession
-	 * @param \OCP\IRequest $request
+	 * @param string $appName
+	 * @param IRequest $request
+	 * @param IGroupManager $groupManager
+	 * @param IUserSession $userSession
 	 */
-	public function __construct(\OCP\IGroupManager $groupManager,
-								\OCP\IUserSession $userSession,
-								\OCP\IRequest $request) {
+	public function __construct(
+			$appName,
+			IRequest $request,
+			IGroupManager $groupManager,
+			IUserSession $userSession) {
+		parent::__construct($appName, $request);
+
 		$this->groupManager = $groupManager;
 		$this->userSession = $userSession;
-		$this->request = $request;
 	}
 
 	/**
 	 * returns a list of groups
 	 *
-	 * @param array $parameters
-	 * @return \OC\OCS\Result
+	 * @NoAdminRequired
+	 *
+	 * @param string $search
+	 * @param int $limit
+	 * @param int $offset
+	 * @return DataResponse
 	 */
-	public function getGroups($parameters) {
-		$search = $this->request->getParam('search', '');
-		$limit = $this->request->getParam('limit');
-		$offset = $this->request->getParam('offset');
-
+	public function getGroups($search = '', $limit = null, $offset = null) {
 		if ($limit !== null) {
 			$limit = (int)$limit;
 		}
@@ -76,27 +84,24 @@ class Groups{
 			return $group->getGID();
 		}, $groups);
 
-		return new \OC\OCS\Result(['groups' => $groups]);
+		return new DataResponse(['groups' => $groups]);
 	}
 
 	/**
 	 * returns an array of users in the group specified
 	 *
-	 * @param array $parameters
-	 * @return \OC\OCS\Result
+	 * @NoAdminRequired
+	 *
+	 * @param string $groupId
+	 * @return DataResponse
+	 * @throws OCSException
 	 */
-	public function getGroup($parameters) {
-		// Check if user is logged in
+	public function getGroup($groupId) {
 		$user = $this->userSession->getUser();
-		if ($user === null) {
-			return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED);
-		}
-
-		$groupId = $parameters['groupid'];
 
 		// Check the group exists
 		if(!$this->groupManager->groupExists($groupId)) {
-			return new \OC\OCS\Result(null, \OCP\API::RESPOND_NOT_FOUND, 'The requested group could not be found');
+			throw new OCSException('The requested group could not be found', \OCP\API::RESPOND_NOT_FOUND);
 		}
 
 		$isSubadminOfGroup = false;
@@ -114,59 +119,62 @@ class Groups{
 				return $user->getUID();
 			}, $users);
 			$users = array_values($users);
-			return new \OC\OCS\Result(['users' => $users]);
+			return new DataResponse(['users' => $users]);
 		} else {
-			return new \OC\OCS\Result(null, \OCP\API::RESPOND_UNAUTHORISED, 'User does not have access to specified group');
+			throw new OCSException('User does not have access to specified group', \OCP\API::RESPOND_UNAUTHORISED);
 		}
 	}
 
 	/**
 	 * creates a new group
 	 *
-	 * @param array $parameters
-	 * @return \OC\OCS\Result
+	 * @NoAdminRequired
+	 *
+	 * @param string $groupid
+	 * @return DataResponse
+	 * @throws OCSException
 	 */
-	public function addGroup($parameters) {
+	public function addGroup($groupid) {
 		// Validate name
-		$groupId = $this->request->getParam('groupid', '');
-		if(empty($groupId)){
+		if(empty($groupid)){
 			\OCP\Util::writeLog('provisioning_api', 'Group name not supplied', \OCP\Util::ERROR);
-			return new \OC\OCS\Result(null, 101, 'Invalid group name');
+			throw new OCSException('Invalid group name', 101);
 		}
 		// Check if it exists
-		if($this->groupManager->groupExists($groupId)){
-			return new \OC\OCS\Result(null, 102);
+		if($this->groupManager->groupExists($groupid)){
+			throw new OCSException('', 102);
 		}
-		$this->groupManager->createGroup($groupId);
-		return new \OC\OCS\Result(null, 100);
+		$this->groupManager->createGroup($groupid);
+		return new DataResponse();
 	}
 
 	/**
-	 * @param array $parameters
-	 * @return \OC\OCS\Result
+	 * @param string $groupId
+	 * @return DataResponse
+	 * @throws OCSException
 	 */
-	public function deleteGroup($parameters) {
+	public function deleteGroup($groupId) {
 		// Check it exists
-		if(!$this->groupManager->groupExists($parameters['groupid'])){
-			return new \OC\OCS\Result(null, 101);
-		} else if($parameters['groupid'] === 'admin' || !$this->groupManager->get($parameters['groupid'])->delete()){
+		if(!$this->groupManager->groupExists($groupId)){
+			throw new OCSException('', 101);
+		} else if($groupId === 'admin' || !$this->groupManager->get($groupId)->delete()){
 			// Cannot delete admin group
-			return new \OC\OCS\Result(null, 102);
-		} else {
-			return new \OC\OCS\Result(null, 100);
+			throw new OCSException('', 102);
 		}
+
+		return new DataResponse(null, 100);
 	}
 
 	/**
-	 * @param array $parameters
-	 * @return \OC\OCS\Result
+	 * @param string $groupId
+	 * @return DataResponse
+	 * @throws OCSException
 	 */
-	public function getSubAdminsOfGroup($parameters) {
-		$group = $parameters['groupid'];
+	public function getSubAdminsOfGroup($groupId) {
 		// Check group exists
-		$targetGroup = $this->groupManager->get($group);
+		$targetGroup = $this->groupManager->get($groupId);
 		if($targetGroup === null) {
-			return new \OC\OCS\Result(null, 101, 'Group does not exist');
+			throw new OCSException('Group does not exist', 101);
 		}
 
 		$subadmins = $this->groupManager->getSubAdmin()->getGroupsSubAdmins($targetGroup);
@@ -176,7 +184,7 @@ class Groups{
 			$uids[] = $user->getUID();
 		}
 
-		return new \OC\OCS\Result($uids);
+		return new DataResponse($uids);
 	}
 
 }
diff --git a/apps/provisioning_api/tests/GroupsTest.php b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php
index 2fa19c4f8b4..25059e85425 100644
--- a/apps/provisioning_api/tests/GroupsTest.php
+++ b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php
@@ -24,24 +24,20 @@
  *
  */
 
-namespace OCA\Provisioning_API\Tests;
+namespace OCA\Provisioning_API\Tests\Controller;
 
-use OCA\Provisioning_API\Groups;
-use OCP\API;
+use OCA\Provisioning_API\Controller\GroupsController;
 use OCP\IGroupManager;
 use OCP\IUserSession;
-use OCP\IRequest;
 
-class GroupsTest extends \Test\TestCase {
+class GroupsControllerTest extends \Test\TestCase {
 	/** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */
 	protected $groupManager;
 	/** @var IUserSession|\PHPUnit_Framework_MockObject_MockObject */
 	protected $userSession;
-	/** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */
-	protected $request;
 	/** @var \OC\SubAdmin|\PHPUnit_Framework_MockObject_MockObject */
 	protected $subAdminManager;
-	/** @var Groups */
+	/** @var GroupsController */
 	protected $api;
 
 	protected function setUp() {
@@ -61,13 +57,14 @@ class GroupsTest extends \Test\TestCase {
 		$this->userSession = $this->getMockBuilder('OCP\IUserSession')
 			->disableOriginalConstructor()
 			->getMock();
-		$this->request = $this->getMockBuilder('OCP\IRequest')
+		$request = $this->getMockBuilder('OCP\IRequest')
 			->disableOriginalConstructor()
 			->getMock();
-		$this->api = new Groups(
+		$this->api = new GroupsController(
+			'provisioning_api',
+			$request,
 			$this->groupManager,
-			$this->userSession,
-			$this->request
+			$this->userSession
 		);
 	}
 
@@ -148,15 +145,6 @@ class GroupsTest extends \Test\TestCase {
 	 * @param int|null $offset
 	 */
 	public function testGetGroups($search, $limit, $offset) {
-		$this->request
-			->expects($this->exactly(3))
-			->method('getParam')
-			->will($this->returnValueMap([
-				['search', '', $search],
-				['limit', null, $limit],
-				['offset', null, $offset],
-			]));
-
 		$groups = [$this->createGroup('group1'), $this->createGroup('group2')];
 
 		$search = $search === null ? '' : $search;
@@ -167,19 +155,8 @@ class GroupsTest extends \Test\TestCase {
 			->with($search, $limit, $offset)
 			->willReturn($groups);
 
-		$result = $this->api->getGroups([]);
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertTrue($result->succeeded());
-		$this->assertEquals(['group1', 'group2'], $result->getData()['groups']);
-	}
-
-	public function testGetGroupAsUser() {
-		$result = $this->api->getGroup([]);
-
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertFalse($result->succeeded());
-		$this->assertEquals(API::RESPOND_UNAUTHORISED, $result->getStatusCode());
-
+		$result = $this->api->getGroups($search, $limit, $offset);
+		$this->assertEquals(['groups' => ['group1', 'group2']], $result->getData());
 	}
 
 	public function testGetGroupAsSubadmin() {
@@ -201,17 +178,15 @@ class GroupsTest extends \Test\TestCase {
 				$this->createUser('user2')
 			]);
 
-		$result = $this->api->getGroup([
-			'groupid' => 'group',
-		]);
+		$result = $this->api->getGroup('group');
 
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertTrue($result->succeeded());
-		$this->assertEquals(1, sizeof($result->getData()), 'Asserting the result data array only has the "users" key');
-		$this->assertArrayHasKey('users', $result->getData());
-		$this->assertEquals(['user1', 'user2'], $result->getData()['users']);
+		$this->assertEquals(['users' => ['user1', 'user2']], $result->getData());
 	}
 
+	/**
+	 * @expectedException \OCP\AppFramework\OCS\OCSException
+	 * @expectedExceptionCode 997
+	 */
 	public function testGetGroupAsIrrelevantSubadmin() {
 		$group = $this->createGroup('group');
 		$otherGroup = $this->createGroup('otherGroup');
@@ -226,13 +201,7 @@ class GroupsTest extends \Test\TestCase {
 			->with('group')
 			->willReturn(true);
 
-		$result = $this->api->getGroup([
-			'groupid' => 'group',
-		]);
-
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertFalse($result->succeeded());
-		$this->assertEquals(API::RESPOND_UNAUTHORISED, $result->getStatusCode());
+		$this->api->getGroup('group');
 	}
 
 	public function testGetGroupAsAdmin() {
@@ -254,39 +223,29 @@ class GroupsTest extends \Test\TestCase {
 				$this->createUser('user2')
 			]);
 
-		$result = $this->api->getGroup([
-			'groupid' => 'group',
-		]);
+		$result = $this->api->getGroup('group');
 
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertTrue($result->succeeded());
-		$this->assertEquals(1, sizeof($result->getData()), 'Asserting the result data array only has the "users" key');
-		$this->assertArrayHasKey('users', $result->getData());
-		$this->assertEquals(['user1', 'user2'], $result->getData()['users']);
+		$this->assertEquals(['users' => ['user1', 'user2']], $result->getData());
 	}
 
+	/**
+	 * @expectedException \OCP\AppFramework\OCS\OCSException
+	 * @expectedExceptionCode 998
+	 * @expectedExceptionMessage The requested group could not be found
+	 */
 	public function testGetGroupNonExisting() {
 		$this->asUser();
 
-		$result = $this->api->getGroup([
-			'groupid' => $this->getUniqueID()
-		]);
-
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertFalse($result->succeeded());
-		$this->assertEquals(API::RESPOND_NOT_FOUND, $result->getStatusCode());
-		$this->assertEquals('The requested group could not be found', $result->getMeta()['message']);
+		$this->api->getGroup($this->getUniqueID());
 	}
 
+	/**
+	 * @expectedException \OCP\AppFramework\OCS\OCSException
+	 * @expectedExceptionCode 101
+	 * @expectedExceptionMessage Group does not exist
+	 */
 	public function testGetSubAdminsOfGroupsNotExists() {
-		$result = $this->api->getSubAdminsOfGroup([
-			'groupid' => 'NonExistingGroup',
-		]);
-
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertFalse($result->succeeded());
-		$this->assertEquals(101, $result->getStatusCode());
-		$this->assertEquals('Group does not exist', $result->getMeta()['message']);
+		$this->api->getSubAdminsOfGroup('NonExistingGroup');
 	}
 
 	public function testGetSubAdminsOfGroup() {
@@ -305,12 +264,7 @@ class GroupsTest extends \Test\TestCase {
 				$this->createUser('SubAdmin2'),
 			]);
 
-		$result = $this->api->getSubAdminsOfGroup([
-			'groupid' => 'GroupWithSubAdmins',
-		]);
-
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertTrue($result->succeeded());
+		$result = $this->api->getSubAdminsOfGroup('GroupWithSubAdmins');
 		$this->assertEquals(['SubAdmin1', 'SubAdmin2'], $result->getData());
 	}
 
@@ -328,53 +282,33 @@ class GroupsTest extends \Test\TestCase {
 			->willReturn([
 			]);
 
-		$result = $this->api->getSubAdminsOfGroup([
-			'groupid' => 'GroupWithOutSubAdmins',
-		]);
-
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertTrue($result->succeeded());
+		$result = $this->api->getSubAdminsOfGroup('GroupWithOutSubAdmins');
 		$this->assertEquals([], $result->getData());
 	}
 
+	/**
+	 * @expectedException \OCP\AppFramework\OCS\OCSException
+	 * @expectedExceptionCode 101
+	 * @expectedExceptionMessage Invalid group name
+	 */
 	public function testAddGroupEmptyGroup() {
-		$this->request
-			->method('getParam')
-			->with('groupid')
-			->willReturn('');
-
-		$result = $this->api->addGroup([]);
-
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertFalse($result->succeeded());
-		$this->assertEquals(101, $result->getStatusCode());
-		$this->assertEquals('Invalid group name', $result->getMeta()['message']);
+		$this->api->addGroup('');
 	}
 
+	/**
+	 * @expectedException \OCP\AppFramework\OCS\OCSException
+	 * @expectedExceptionCode 102
+	 */
 	public function testAddGroupExistingGroup() {
-		$this->request
-			->method('getParam')
-			->with('groupid')
-			->willReturn('ExistingGroup');
-
 		$this->groupManager
 			->method('groupExists')
 			->with('ExistingGroup')
 			->willReturn(true);
 
-		$result = $this->api->addGroup([]);
-
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertFalse($result->succeeded());
-		$this->assertEquals(102, $result->getStatusCode());
+		$this->api->addGroup('ExistingGroup');
 	}
 
 	public function testAddGroup() {
-		$this->request
-			->method('getParam')
-			->with('groupid')
-			->willReturn('NewGroup');
-
 		$this->groupManager
 			->method('groupExists')
 			->with('NewGroup')
@@ -385,17 +319,10 @@ class GroupsTest extends \Test\TestCase {
 			->method('createGroup')
 			->with('NewGroup');
 
-		$result = $this->api->addGroup([]);
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertTrue($result->succeeded());
+		$this->api->addGroup('NewGroup');
 	}
 
 	public function testAddGroupWithSpecialChar() {
-		$this->request
-			->method('getParam')
-			->with('groupid')
-			->willReturn('Iñtërnâtiônàlizætiøn');
-
 		$this->groupManager
 			->method('groupExists')
 			->with('Iñtërnâtiônàlizætiøn')
@@ -406,32 +333,28 @@ class GroupsTest extends \Test\TestCase {
 			->method('createGroup')
 			->with('Iñtërnâtiônàlizætiøn');
 
-		$result = $this->api->addGroup([]);
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertTrue($result->succeeded());
+		$this->api->addGroup('Iñtërnâtiônàlizætiøn');
 	}
 
+	/**
+	 * @expectedException \OCP\AppFramework\OCS\OCSException
+	 * @expectedExceptionCode 101
+	 */
 	public function testDeleteGroupNonExisting() {
-		$result = $this->api->deleteGroup([
-			'groupid' => 'NonExistingGroup'
-		]);
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertFalse($result->succeeded());
-		$this->assertEquals(101, $result->getStatusCode());
+		$this->api->deleteGroup('NonExistingGroup');
 	}
 
+	/**
+	 * @expectedException \OCP\AppFramework\OCS\OCSException
+	 * @expectedExceptionCode 102
+	 */
 	public function testDeleteAdminGroup() {
 		$this->groupManager
 			->method('groupExists')
 			->with('admin')
 			->willReturn('true');
 
-		$result = $this->api->deleteGroup([
-			'groupid' => 'admin'
-		]);
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertFalse($result->succeeded());
-		$this->assertEquals(102, $result->getStatusCode());
+		$this->api->deleteGroup('admin');
 	}
 
 	public function testDeleteGroup() {
@@ -450,10 +373,6 @@ class GroupsTest extends \Test\TestCase {
 			->method('delete')
 			->willReturn(true);
 
-		$result = $this->api->deleteGroup([
-			'groupid' => 'ExistingGroup',
-		]);
-		$this->assertInstanceOf('\OC\OCS\Result', $result);
-		$this->assertTrue($result->succeeded());
+		$this->api->deleteGroup('ExistingGroup');
 	}
 }