Merge pull request #20292 from owncloud/provisioning_api_group_unittest

Moved grouptest over to pure unit tests

apps/provisioning_api/appinfo/routes.php

@@ -51,7 +51,8 @@ API::register('get', '/cloud/users/{userid}/subadmins', [$users, 'getUserSubAdmi
 // Groups
 $groups = new \OCA\Provisioning_API\Groups(
 	\OC::$server->getGroupManager(),
-	\OC::$server->getUserSession()
+	\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);

apps/provisioning_api/lib/groups.php

@@ -37,14 +37,20 @@ class Groups{
 	/** @var \OCP\IUserSession */
 	private $userSession;
 
+	/** @var \OCP\IRequest */
+	private $request;
+
 	/**
 	 * @param \OCP\IGroupManager $groupManager
 	 * @param \OCP\IUserSession $userSession
+	 * @param \OCP\IRequest $request
 	 */
 	public function __construct(\OCP\IGroupManager $groupManager,
-								\OCP\IUserSession $userSession) {
+								\OCP\IUserSession $userSession,
+								\OCP\IRequest $request) {
 		$this->groupManager = $groupManager;
 		$this->userSession = $userSession;
+		$this->request = $request;
 	}
 
 	/**
@@ -54,9 +60,16 @@ class Groups{
 	 * @return OC_OCS_Result
 	 */
 	public function getGroups($parameters) {
-		$search = !empty($_GET['search']) ? $_GET['search'] : '';
-		$limit = !empty($_GET['limit']) ? $_GET['limit'] : null;
-		$offset = !empty($_GET['offset']) ? $_GET['offset'] : null;
+		$search = $this->request->getParam('search', '');
+		$limit = $this->request->getParam('limit');
+		$offset = $this->request->getParam('offset');
+
+		if ($limit !== null) {
+			$limit = (int)$limit;
+		}
+		if ($offset !== null) {
+			$offset = (int)$offset;
+		}
 
 		$groups = $this->groupManager->search($search, $limit, $offset);
 		$groups = array_map(function($group) {
@@ -80,21 +93,23 @@ class Groups{
 			return new OC_OCS_Result(null, \OCP\API::RESPOND_UNAUTHORISED);
 		}
 
+		$groupId = $parameters['groupid'];
+
 		// Check the group exists
-		if(!$this->groupManager->groupExists($parameters['groupid'])) {
+		if(!$this->groupManager->groupExists($groupId)) {
 			return new OC_OCS_Result(null, \OCP\API::RESPOND_NOT_FOUND, 'The requested group could not be found');
 		}
 
 		$isSubadminOfGroup = false;
-		$targetGroupObject =$this->groupManager->get($parameters['groupid']);
-		if($targetGroupObject !== null) {
-			$isSubadminOfGroup =$this->groupManager->getSubAdmin()->isSubAdminofGroup($user, $targetGroupObject);
+		$group = $this->groupManager->get($groupId);
+		if ($group !== null) {
+			$isSubadminOfGroup =$this->groupManager->getSubAdmin()->isSubAdminofGroup($user, $group);
 		}
 
 		// Check subadmin has access to this group
 		if($this->groupManager->isAdmin($user->getUID())
 		   || $isSubadminOfGroup) {
-			$users = $this->groupManager->get($parameters['groupid'])->getUsers();
+			$users = $this->groupManager->get($groupId)->getUsers();
 			$users =  array_map(function($user) {
 				/** @var IUser $user */
 				return $user->getUID();
@@ -114,7 +129,7 @@ class Groups{
 	 */
 	public function addGroup($parameters) {
 		// Validate name
-		$groupId = isset($_POST['groupid']) ? $_POST['groupid'] : '';
+		$groupId = $this->request->getParam('groupid', '');
 		if( preg_match( '/[^a-zA-Z0-9 _\.@\-]/', $groupId ) || empty($groupId)){
 			\OCP\Util::writeLog('provisioning_api', 'Attempt made to create group using invalid characters.', \OCP\Util::ERROR);
 			return new OC_OCS_Result(null, 101, 'Invalid group name');

apps/provisioning_api/tests/groupstest.php

@@ -25,79 +25,131 @@
 
 namespace OCA\Provisioning_API\Tests;
 
-use OCP\IUserManager;
 use OCP\IGroupManager;
 use OCP\IUserSession;
+use OCP\IRequest;
 
-class GroupsTest extends TestCase {
-	/** @var IUserManager */
-	protected $userManager;
+class GroupsTest extends \Test\TestCase {
 	/** @var IGroupManager */
 	protected $groupManager;
 	/** @var IUserSession */
 	protected $userSession;
+	/** @var IRequest */
+	protected $request;
+	/** @var \OC\SubAdmin */
+	protected $subAdminManager;
 	/** @var \OCA\Provisioning_API\Groups */
 	protected $api;
 
 	protected function setup() {
-		parent::setup();
+		$this->subAdminManager = $this->getMockBuilder('OC\SubAdmin')->disableOriginalConstructor()->getMock();
 
-		$this->userManager = \OC::$server->getUserManager();
-		$this->groupManager = \OC::$server->getGroupManager();
-		$this->userSession = \OC::$server->getUserSession();
+		$this->groupManager = $this->getMockBuilder('OC\Group\Manager')->disableOriginalConstructor()->getMock();
+		$this->groupManager
+			->method('getSubAdmin')
+			->willReturn($this->subAdminManager);
+
+		$this->userSession = $this->getMock('OCP\IUserSession');
+		$this->request = $this->getMock('OCP\IRequest');
 		$this->api = new \OCA\Provisioning_API\Groups(
 			$this->groupManager,
-			$this->userSession
+			$this->userSession,
+			$this->request
 		);
 	}
 
-	public function testGetGroups() {
-		$groups = [];
-		$id = $this->getUniqueID();
+	private function createGroup($gid) {
+		$group = $this->getMock('OCP\IGroup');
+		$group
+			->method('getGID')
+			->willReturn($gid);
+		return $group;
+	}
 
-		for ($i=0; $i < 10; $i++) {
-			$groups[] = $this->groupManager->createGroup($id . '_' . $i);
-		}
+	private function createUser($uid) {
+		$user = $this->getMock('OCP\IUser');
+		$user
+			->method('getUID')
+			->willReturn($uid);
+		return $user;
+	}
 
-		$_GET = [];
-		$result = $this->api->getGroups([]);
-		$this->assertInstanceOf('OC_OCS_Result', $result);
-		$this->assertTrue($result->succeeded());
-		$this->assertCount(count($this->groupManager->search('')), $result->getData()['groups']);
-		$this->assertContains('admin', $result->getData()['groups']);
-		foreach ($groups as $group) {
-			$this->assertContains($group->getGID(), $result->getData()['groups']);
-		}
+	private function asUser() {
+		$user = $this->createUser('user');
+		$this->userSession
+			->method('getUser')
+			->willReturn($user);
+	}
 
-		$_GET = [
-			'search' => $id,
-			'limit' => 5,
-			'offset' => 2
+	private function asAdmin() {
+		$user = $this->createUser('admin');
+		$this->userSession
+			->method('getUser')
+			->willReturn($user);
+
+		$this->groupManager
+			->method('isAdmin')
+			->with('admin')
+			->willReturn(true);
+	}
+
+	private function asSubAdminOfGroup($group) {
+		$user = $this->createUser('subAdmin');
+		$this->userSession
+			->method('getUser')
+			->willReturn($user);
+
+		$this->subAdminManager
+			->method('isSubAdminOfGroup')
+			->will($this->returnCallback(function($_user, $_group) use ($user, $group) {
+				if ($_user === $user && $_group === $group) {
+					return true;
+				}
+				return false;
+			}));
+	}
+
+	public function dataGetGroups() {
+		return [
+			[null, null, null],
+			['foo', null, null],
+			[null, 1, null],
+			[null, null, 2],
+			['foo', 1, 2],
 		];
+	}
+
+	/**
+	 * @dataProvider dataGetGroups
+	 */
+	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;
+
+		$this->groupManager
+			->expects($this->once())
+			->method('search')
+			->with($search, $limit, $offset)
+			->willReturn($groups);
+
 		$result = $this->api->getGroups([]);
 		$this->assertInstanceOf('OC_OCS_Result', $result);
 		$this->assertTrue($result->succeeded());
-		$this->assertCount(5, $result->getData()['groups']);
-		foreach (array_splice($groups, 2, 5) as $group) {
-			$this->assertContains($group->getGID(), $result->getData()['groups']);
-		}
-
-		foreach ($groups as $group) {
-			$group->delete();
-		}
+		$this->assertEquals(['group1', 'group2'], $result->getData()['groups']);
 	}
 
 	public function testGetGroupAsUser() {
-
-		$users = $this->generateUsers(2);
-		$this->userSession->setUser($users[0]);
-
-		$group = $this->groupManager->createGroup($this->getUniqueID());
-		$group->addUser($users[1]);
-
-		$result = $this->api->getGroup(array(
-			'groupid' => $group->getGID(),
-		));
+		$result = $this->api->getGroup([]);
 
 		$this->assertInstanceOf('OC_OCS_Result', $result);
 		$this->assertFalse($result->succeeded());
@@ -106,80 +158,91 @@ class GroupsTest extends TestCase {
 	}
 
 	public function testGetGroupAsSubadmin() {
+		$group = $this->createGroup('group');
+		$this->asSubAdminOfGroup($group);
 
-		$users = $this->generateUsers(2);
-		$this->userSession->setUser($users[0]);
-
-		$group = $this->groupManager->createGroup($this->getUniqueID());
-		$group->addUser($users[0]);
-		$group->addUser($users[1]);
-
-		$this->groupManager->getSubAdmin()->createSubAdmin($users[0], $group);
+		$this->groupManager
+			->method('get')
+			->with('group')
+			->willReturn($group);
+		$this->groupManager
+			->method('groupExists')
+			->with('group')
+			->willReturn(true);
+		$group
+			->method('getUsers')
+			->willReturn([
+				$this->createUser('user1'),
+				$this->createUser('user2')
+			]);
 
 		$result = $this->api->getGroup([
-			'groupid' => $group->getGID(),
+			'groupid' => '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());
-		$resultData = $result->getData();
-		$resultData = $resultData['users'];
-
-		$users = array_map(function($user) {
-			return $user->getUID();
-		}, $users);
-
-		sort($users);
-		sort($resultData);
-		$this->assertEquals($users, $resultData);
-
+		$this->assertEquals(['user1', 'user2'], $result->getData()['users']);
 	}
 
 	public function testGetGroupAsIrrelevantSubadmin() {
+		$group = $this->createGroup('group');
+		$otherGroup = $this->createGroup('otherGroup');
+		$this->asSubAdminOfGroup($otherGroup);
 
-		$users = $this->generateUsers(2);
-		$this->userSession->setUser($users[0]);
-
-		$group1 = $this->groupManager->createGroup($this->getUniqueID());
-		$group2 = $this->groupManager->createGroup($this->getUniqueID());
-		$group1->addUser($users[1]);
-		$group2->addUser($users[0]);
-
-		$this->groupManager->getSubAdmin()->createSubAdmin($users[0], $group2);
+		$this->groupManager
+			->method('get')
+			->with('group')
+			->willReturn($group);
+		$this->groupManager
+			->method('groupExists')
+			->with('group')
+			->willReturn(true);
 
 		$result = $this->api->getGroup([
-			'groupid' => $group1->getGID(),
+			'groupid' => 'group',
 		]);
 
 		$this->assertInstanceOf('OC_OCS_Result', $result);
 		$this->assertFalse($result->succeeded());
 		$this->assertEquals(\OCP\API::RESPOND_UNAUTHORISED, $result->getStatusCode());
-
 	}
 
 	public function testGetGroupAsAdmin() {
+		$group = $this->createGroup('group');
+		$this->asAdmin();
 
-		$users = $this->generateUsers(2);
-		$this->userSession->setUser($users[0]);
-
-		$group = $this->groupManager->createGroup($this->getUniqueID());
-
-		$group->addUser($users[1]);
-		$this->groupManager->get('admin')->addUser($users[0]);
+		$this->groupManager
+			->method('get')
+			->with('group')
+			->willReturn($group);
+		$this->groupManager
+			->method('groupExists')
+			->with('group')
+			->willReturn(true);
+		$group
+			->method('getUsers')
+			->willReturn([
+				$this->createUser('user1'),
+				$this->createUser('user2')
+			]);
 
 		$result = $this->api->getGroup([
-			'groupid' => $group->getGID(),
+			'groupid' => 'group',
 		]);
 
 		$this->assertInstanceOf('OC_OCS_Result', $result);
 		$this->assertTrue($result->succeeded());
-		$this->assertEquals(['users' => [$users[1]->getUID()]], $result->getData());
-
+		$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']);
 	}
 
 	public function testGetGroupNonExisting() {
+		$this->asUser();
+
 		$result = $this->api->getGroup([
 			'groupid' => $this->getUniqueId()
 		]);
@@ -190,35 +253,72 @@ class GroupsTest extends TestCase {
 		$this->assertEquals('The requested group could not be found', $result->getMeta()['message']);
 	}
 
-	public function testGetSubAdminsOfGroup() {
-		$user1 = $this->generateUsers();
-		$user2 = $this->generateUsers();
-		$this->userSession->setUser($user1);
-		$this->groupManager->get('admin')->addUser($user1);
-		$group1 = $this->groupManager->createGroup($this->getUniqueID());
-		$this->groupManager->getSubAdmin()->createSubAdmin($user2, $group1);
+	public function testGetSubAdminsOfGroupsNotExists() {
 		$result = $this->api->getSubAdminsOfGroup([
-			'groupid' => $group1->getGID(),
+			'groupid' => 'NonExistingGroup',
 		]);
-		$this->assertInstanceOf('OC_OCS_Result', $result);
-		$this->assertTrue($result->succeeded());
-		$data = $result->getData();
-		$this->assertEquals($user2->getUID(), reset($data));
-		$group1->delete();
 
-		$user1 = $this->generateUsers();
-		$this->userSession->setUser($user1);
-		$this->groupManager->get('admin')->addUser($user1);
-		$result = $this->api->getSubAdminsOfGroup([
-			'groupid' => $this->getUniqueID(),
-		]);
 		$this->assertInstanceOf('OC_OCS_Result', $result);
 		$this->assertFalse($result->succeeded());
 		$this->assertEquals(101, $result->getStatusCode());
+		$this->assertEquals('Group does not exist', $result->getMeta()['message']);
+	}
+
+	public function testGetSubAdminsOfGroup() {
+		$group = $this->createGroup('GroupWithSubAdmins');
+		$this->groupManager
+			->method('get')
+			->with('GroupWithSubAdmins')
+			->willReturn($group);
+
+		$this->subAdminManager
+			->expects($this->once())
+			->method('getGroupsSubAdmins')
+			->with($group)
+			->willReturn([
+				$this->createUser('SubAdmin1'),
+				$this->createUser('SubAdmin2'),
+			]);
+
+		$result = $this->api->getSubAdminsOfGroup([
+			'groupid' => 'GroupWithSubAdmins',
+		]);
+
+		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$this->assertTrue($result->succeeded());
+		$this->assertEquals(['SubAdmin1', 'SubAdmin2'], $result->getData());
+	}
+
+	public function testGetSubAdminsOfGroupEmptyList() {
+		$group = $this->createGroup('GroupWithOutSubAdmins');
+		$this->groupManager
+			->method('get')
+			->with('GroupWithOutSubAdmins')
+			->willReturn($group);
+
+		$this->subAdminManager
+			->expects($this->once())
+			->method('getGroupsSubAdmins')
+			->with($group)
+			->willReturn([
+			]);
+
+		$result = $this->api->getSubAdminsOfGroup([
+			'groupid' => 'GroupWithOutSubAdmins',
+		]);
+
+		$this->assertInstanceOf('OC_OCS_Result', $result);
+		$this->assertFalse($result->succeeded());
+		$this->assertEquals(102, $result->getStatusCode());
+		$this->assertEquals('Unknown error occured', $result->getMeta()['message']);
 	}
 
 	public function testAddGroupEmptyGroup() {
-		$_POST = [];
+		$this->request
+			->method('getParam')
+			->with('groupid')
+			->willReturn('');
+
 		$result = $this->api->addGroup([]);
 
 		$this->assertInstanceOf('OC_OCS_Result', $result);
@@ -228,40 +328,47 @@ class GroupsTest extends TestCase {
 	}
 
 	public function testAddGroupExistingGroup() {
-		$group = $this->groupManager->createGroup($this->getUniqueID());
+		$this->request
+			->method('getParam')
+			->with('groupid')
+			->willReturn('ExistingGroup');
+
+		$this->groupManager
+			->method('groupExists')
+			->with('ExistingGroup')
+			->willReturn(true);
 
-		$_POST = [
-			'groupid' => $group->getGID()
-		];
 		$result = $this->api->addGroup([]);
 
 		$this->assertInstanceOf('OC_OCS_Result', $result);
 		$this->assertFalse($result->succeeded());
 		$this->assertEquals(102, $result->getStatusCode());
-
-		$group->delete();
 	}
 
 	public function testAddGroup() {
-		$group = $this->getUniqueId();
+		$this->request
+			->method('getParam')
+			->with('groupid')
+			->willReturn('NewGroup');
 
-		$_POST = [ 
-			'groupid' => $group
-		];
+		$this->groupManager
+			->method('groupExists')
+			->with('NewGroup')
+			->willReturn(false);
+
+		$this->groupManager
+			->expects($this->once())
+			->method('createGroup')
+			->with('NewGroup');
 
 		$result = $this->api->addGroup([]);
 		$this->assertInstanceOf('OC_OCS_Result', $result);
 		$this->assertTrue($result->succeeded());
-		$this->assertTrue($this->groupManager->groupExists($group));
-
-		$this->groupManager->get($group)->delete();
 	}
 
 	public function testDeleteGroupNonExisting() {
-		$group = $this->getUniqueId();
-
 		$result = $this->api->deleteGroup([
-			'groupid' => $group
+			'groupid' => 'NonExistingGroup'
 		]);
 		$this->assertInstanceOf('OC_OCS_Result', $result);
 		$this->assertFalse($result->succeeded());
@@ -269,6 +376,11 @@ class GroupsTest extends TestCase {
 	}
 
 	public function testDeleteAdminGroup() {
+		$this->groupManager
+			->method('groupExists')
+			->with('admin')
+			->willReturn('true');
+
 		$result = $this->api->deleteGroup([
 			'groupid' => 'admin'
 		]);
@@ -278,13 +390,25 @@ class GroupsTest extends TestCase {
 	}
 
 	public function testDeleteGroup() {
-		$group = $this->groupManager->createGroup($this->getUniqueId());
+		$this->groupManager
+			->method('groupExists')
+			->with('ExistingGroup')
+			->willReturn('true');
+
+		$group = $this->createGroup('ExistingGroup');
+		$this->groupManager
+			->method('get')
+			->with('ExistingGroup')
+			->willReturn($group);
+		$group
+			->expects($this->once())
+			->method('delete')
+			->willReturn(true);
 
 		$result = $this->api->deleteGroup([
-			'groupid' => $group->getGID()
+			'groupid' => 'ExistingGroup',
 		]);
 		$this->assertInstanceOf('OC_OCS_Result', $result);
 		$this->assertTrue($result->succeeded());
-		$this->assertFalse($this->groupManager->groupExists($group->getGID()));
 	}
 }