From 432e7c93c6dad564abbaec1e3d374f73653d7ba6 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 11 Aug 2016 09:46:25 +0200 Subject: [PATCH] Move Groups over to OCSController * Take advantage of the AppFramework * Fix tests --- apps/provisioning_api/appinfo/routes.php | 27 ++- .../GroupsController.php} | 128 ++++++------ .../GroupsControllerTest.php} | 197 ++++++------------ 3 files changed, 138 insertions(+), 214 deletions(-) rename apps/provisioning_api/lib/{Groups.php => Controller/GroupsController.php} (52%) rename apps/provisioning_api/tests/{GroupsTest.php => Controller/GroupsControllerTest.php} (59%) 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 similarity index 52% rename from apps/provisioning_api/lib/Groups.php rename to 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 similarity index 59% rename from apps/provisioning_api/tests/GroupsTest.php rename to 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'); } } -- 2.39.5