From 7e7dd92f6b7c255ea7a94cbcf0e2e762ef49f8ee Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Fri, 30 Jan 2015 14:16:16 +0100 Subject: [PATCH] Add unit tests --- settings/application.php | 9 +- settings/controller/userscontroller.php | 24 ++- .../controller/logsettingscontrollertest.php | 1 + .../controller/userscontrollertest.php | 196 +++++++++++++++++- 4 files changed, 211 insertions(+), 19 deletions(-) diff --git a/settings/application.php b/settings/application.php index d5516a1eef..3b2c77ab84 100644 --- a/settings/application.php +++ b/settings/application.php @@ -1,7 +1,7 @@ query('Mail'), $c->query('DefaultMailAddress'), $c->query('URLGenerator'), - $c->query('OCP\\App\\IAppManager') + $c->query('OCP\\App\\IAppManager'), + $c->query('SubAdminOfGroups') ); }); $container->registerService('LogSettingsController', function(IContainer $c) { @@ -145,6 +146,10 @@ class Application extends App { $container->registerService('IsSubAdmin', function(IContainer $c) { return \OC_Subadmin::isSubAdmin(\OC_User::getUser()); }); + /** FIXME: Remove once OC_SubAdmin is non-static and mockable */ + $container->registerService('SubAdminOfGroups', function(IContainer $c) { + return \OC_SubAdmin::getSubAdminsGroups(\OC_User::getUser()); + }); $container->registerService('Mail', function(IContainer $c) { return new \OC_Mail; }); diff --git a/settings/controller/userscontroller.php b/settings/controller/userscontroller.php index fd88692d77..39d94fd2e1 100644 --- a/settings/controller/userscontroller.php +++ b/settings/controller/userscontroller.php @@ -1,7 +1,7 @@ userManager = $userManager; $this->groupManager = $groupManager; @@ -98,6 +103,7 @@ class UsersController extends Controller { $this->mail = $mail; $this->fromMailAddress = $fromMailAddress; $this->urlGenerator = $urlGenerator; + $this->subAdminOfGroups = $subAdminOfGroups; // check for encryption state - TODO see formatUserForIndex $this->isEncryptionAppEnabled = $appManager->isEnabledForUser('files_encryption'); @@ -210,18 +216,15 @@ class UsersController extends Controller { } } else { - /** @var array $subAdminOf List of groups the user is subadmin */ - $subAdminOf = \OC_SubAdmin::getSubAdminsGroups($this->userSession->getUser()->getUID()); - // Set the $gid parameter to an empty value if the subadmin has no rights to access a specific group - if($gid !== '' && !in_array($gid, $subAdminOf)) { + if($gid !== '' && !in_array($gid, $this->subAdminOfGroups)) { $gid = ''; } // Batch all groups the user is subadmin of when a group is specified $batch = []; if($gid === '') { - foreach($subAdminOf as $group) { + foreach($this->subAdminOfGroups as $group) { $groupUsers = $this->groupManager->displayNamesInGroup($group, $pattern, $limit, $offset); foreach($groupUsers as $uid => $displayName) { $batch[$uid] = $displayName; @@ -234,7 +237,10 @@ class UsersController extends Controller { foreach ($batch as $user) { // Only add the groups, this user is a subadmin of - $userGroups = array_intersect($this->groupManager->getUserGroupIds($user), $subAdminOf); + $userGroups = array_values(array_intersect( + $this->groupManager->getUserGroupIds($user), + $this->subAdminOfGroups + )); $users[] = $this->formatUserForIndex($user, $userGroups); } } @@ -274,7 +280,7 @@ class UsersController extends Controller { } } if (empty($groups)) { - $groups = \OC_SubAdmin::getSubAdminsGroups($this->userSession->getUser()->getUID()); + $groups = $this->subAdminOfGroups; } } diff --git a/tests/settings/controller/logsettingscontrollertest.php b/tests/settings/controller/logsettingscontrollertest.php index e80acfa75b..84581bf578 100644 --- a/tests/settings/controller/logsettingscontrollertest.php +++ b/tests/settings/controller/logsettingscontrollertest.php @@ -10,6 +10,7 @@ namespace Test\Settings\Controller; use \OC\Settings\Application; +use OC\Settings\Controller\LogSettingsController; /** * @package OC\Settings\Controller diff --git a/tests/settings/controller/userscontrollertest.php b/tests/settings/controller/userscontrollertest.php index 7dc2d066a5..eb85573d3f 100644 --- a/tests/settings/controller/userscontrollertest.php +++ b/tests/settings/controller/userscontrollertest.php @@ -1,7 +1,7 @@ disableOriginalConstructor()->getMock(); $this->container['Config'] = $this->getMockBuilder('\OCP\IConfig') ->disableOriginalConstructor()->getMock(); - $this->container['IsAdmin'] = true; $this->container['L10N'] ->expects($this->any()) ->method('t') @@ -55,11 +54,9 @@ class UsersControllerTest extends \Test\TestCase { ->disableOriginalConstructor()->getMock(); } - /** - * TODO: Since the function uses the static OC_Subadmin class it can't be mocked - * to test for subadmins. Thus the test always assumes you have admin permissions... - */ - public function testIndex() { + public function testIndexAdmin() { + $this->container['IsAdmin'] = true; + $foo = $this->getMockBuilder('\OC\User\User') ->disableOriginalConstructor()->getMock(); $foo @@ -198,11 +195,167 @@ class UsersControllerTest extends \Test\TestCase { $this->assertEquals($expectedResponse, $response); } + public function testIndexSubAdmin() { + $this->container['IsAdmin'] = false; + $this->container['SubAdminOfGroups'] = ['SubGroup1', 'SubGroup2']; + + $foo = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $foo + ->expects($this->exactly(4)) + ->method('getUID') + ->will($this->returnValue('foo')); + $foo + ->expects($this->once()) + ->method('getDisplayName') + ->will($this->returnValue('M. Foo')); + $foo + ->method('getLastLogin') + ->will($this->returnValue(500)); + $foo + ->method('getHome') + ->will($this->returnValue('/home/foo')); + $foo + ->expects($this->once()) + ->method('getBackendClassName') + ->will($this->returnValue('OC_User_Database')); + $admin = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $admin + ->expects($this->exactly(4)) + ->method('getUID') + ->will($this->returnValue('admin')); + $admin + ->expects($this->once()) + ->method('getDisplayName') + ->will($this->returnValue('S. Admin')); + $admin + ->expects($this->once()) + ->method('getLastLogin') + ->will($this->returnValue(12)); + $admin + ->expects($this->once()) + ->method('getHome') + ->will($this->returnValue('/home/admin')); + $admin + ->expects($this->once()) + ->method('getBackendClassName') + ->will($this->returnValue('OC_User_Dummy')); + $bar = $this->getMockBuilder('\OC\User\User') + ->disableOriginalConstructor()->getMock(); + $bar + ->expects($this->exactly(4)) + ->method('getUID') + ->will($this->returnValue('bar')); + $bar + ->expects($this->once()) + ->method('getDisplayName') + ->will($this->returnValue('B. Ar')); + $bar + ->method('getLastLogin') + ->will($this->returnValue(3999)); + $bar + ->method('getHome') + ->will($this->returnValue('/home/bar')); + $bar + ->expects($this->once()) + ->method('getBackendClassName') + ->will($this->returnValue('OC_User_Dummy')); + + $this->container['GroupManager'] + ->expects($this->at(0)) + ->method('displayNamesInGroup') + ->with('SubGroup1', 'pattern') + ->will($this->returnValue(['foo' => 'M. Foo', 'admin' => 'S. Admin'])); + $this->container['GroupManager'] + ->expects($this->at(1)) + ->method('displayNamesInGroup') + ->with('SubGroup2', 'pattern') + ->will($this->returnValue(['bar' => 'B. Ar'])); + $this->container['GroupManager'] + ->expects($this->exactly(3)) + ->method('getUserGroupIds') + ->will($this->onConsecutiveCalls( + ['SubGroup2', 'SubGroup1'], + ['SubGroup2', 'Foo'], + ['admin', 'SubGroup1', 'testGroup'] + )); + $this->container['UserManager'] + ->expects($this->at(0)) + ->method('get') + ->with('foo') + ->will($this->returnValue($foo)); + $this->container['UserManager'] + ->expects($this->at(1)) + ->method('get') + ->with('admin') + ->will($this->returnValue($admin)); + $this->container['UserManager'] + ->expects($this->at(2)) + ->method('get') + ->with('bar') + ->will($this->returnValue($bar)); + $this->container['Config'] + ->expects($this->exactly(6)) + ->method('getUserValue') + ->will($this->onConsecutiveCalls( + 1024, 'foo@bar.com', + 404, 'admin@bar.com', + 2323, 'bar@dummy.com' + )); + + $expectedResponse = new DataResponse( + [ + 0 => [ + 'name' => 'foo', + 'displayname' => 'M. Foo', + 'groups' => ['SubGroup2', 'SubGroup1'], + 'subadmin' => [], + 'quota' => 1024, + 'storageLocation' => '/home/foo', + 'lastLogin' => 500, + 'backend' => 'OC_User_Database', + 'email' => 'foo@bar.com', + 'isRestoreDisabled' => false, + ], + 1 => [ + 'name' => 'admin', + 'displayname' => 'S. Admin', + 'groups' => ['SubGroup2'], + 'subadmin' => [], + 'quota' => 404, + 'storageLocation' => '/home/admin', + 'lastLogin' => 12, + 'backend' => 'OC_User_Dummy', + 'email' => 'admin@bar.com', + 'isRestoreDisabled' => false, + ], + 2 => [ + 'name' => 'bar', + 'displayname' => 'B. Ar', + 'groups' => ['SubGroup1'], + 'subadmin' => [], + 'quota' => 2323, + 'storageLocation' => '/home/bar', + 'lastLogin' => 3999, + 'backend' => 'OC_User_Dummy', + 'email' => 'bar@dummy.com', + 'isRestoreDisabled' => false, + ], + ] + ); + + $response = $this->container['UsersController']->index(0, 10, '', 'pattern'); + $this->assertEquals($expectedResponse, $response); + } + /** * TODO: Since the function uses the static OC_Subadmin class it can't be mocked * to test for subadmins. Thus the test always assumes you have admin permissions... */ public function testIndexWithSearch() { + $this->container['IsAdmin'] = true; + $foo = $this->getMockBuilder('\OC\User\User') ->disableOriginalConstructor()->getMock(); $foo @@ -326,8 +479,9 @@ class UsersControllerTest extends \Test\TestCase { $this->assertEquals($expectedResponse, $response); } - public function testIndexWithBackend() { + $this->container['IsAdmin'] = true; + $user = $this->getMockBuilder('\OC\User\User') ->disableOriginalConstructor()->getMock(); $user @@ -386,6 +540,8 @@ class UsersControllerTest extends \Test\TestCase { } public function testIndexWithBackendNoUser() { + $this->container['IsAdmin'] = true; + $this->container['UserManager'] ->expects($this->once()) ->method('getBackends') @@ -406,6 +562,8 @@ class UsersControllerTest extends \Test\TestCase { * to test for subadmins. Thus the test always assumes you have admin permissions... */ public function testCreateSuccessfulWithoutGroup() { + $this->container['IsAdmin'] = true; + $user = $this->getMockBuilder('\OC\User\User') ->disableOriginalConstructor()->getMock(); $user @@ -449,6 +607,8 @@ class UsersControllerTest extends \Test\TestCase { * to test for subadmins. Thus the test always assumes you have admin permissions... */ public function testCreateSuccessfulWithGroup() { + $this->container['IsAdmin'] = true; + $user = $this->getMockBuilder('\OC\User\User') ->disableOriginalConstructor()->getMock(); $user @@ -520,6 +680,8 @@ class UsersControllerTest extends \Test\TestCase { * to test for subadmins. Thus the test always assumes you have admin permissions... */ public function testCreateUnsuccessful() { + $this->container['IsAdmin'] = true; + $this->container['UserManager'] ->method('createUser') ->will($this->throwException(new \Exception())); @@ -539,6 +701,8 @@ class UsersControllerTest extends \Test\TestCase { * to test for subadmins. Thus the test always assumes you have admin permissions... */ public function testDestroySelf() { + $this->container['IsAdmin'] = true; + $user = $this->getMockBuilder('\OC\User\User') ->disableOriginalConstructor()->getMock(); $user @@ -567,6 +731,8 @@ class UsersControllerTest extends \Test\TestCase { * to test for subadmins. Thus the test always assumes you have admin permissions... */ public function testDestroy() { + $this->container['IsAdmin'] = true; + $user = $this->getMockBuilder('\OC\User\User') ->disableOriginalConstructor()->getMock(); $user @@ -604,6 +770,8 @@ class UsersControllerTest extends \Test\TestCase { * to test for subadmins. Thus the test always assumes you have admin permissions... */ public function testDestroyUnsuccessful() { + $this->container['IsAdmin'] = true; + $user = $this->getMockBuilder('\OC\User\User') ->disableOriginalConstructor()->getMock(); $user @@ -641,6 +809,8 @@ class UsersControllerTest extends \Test\TestCase { * test if an invalid mail result in a failure response */ public function testCreateUnsuccessfulWithInvalidEMail() { + $this->container['IsAdmin'] = true; + /** * FIXME: Disabled due to missing DI on mail class. * TODO: Re-enable when https://github.com/owncloud/core/pull/12085 is merged. @@ -666,6 +836,8 @@ class UsersControllerTest extends \Test\TestCase { * test if a valid mail result in a successful mail send */ public function testCreateSuccessfulWithValidEMail() { + $this->container['IsAdmin'] = true; + /** * FIXME: Disabled due to missing DI on mail class. * TODO: Re-enable when https://github.com/owncloud/core/pull/12085 is merged. @@ -737,6 +909,8 @@ class UsersControllerTest extends \Test\TestCase { } public function testRestorePossibleWithoutEncryption() { + $this->container['IsAdmin'] = true; + list($user, $expectedResult) = $this->mockUser(); $result = \Test_Helper::invokePrivate($this->container['UsersController'], 'formatUserForIndex', [$user]); @@ -744,6 +918,8 @@ class UsersControllerTest extends \Test\TestCase { } public function testRestorePossibleWithAdminAndUserRestore() { + $this->container['IsAdmin'] = true; + list($user, $expectedResult) = $this->mockUser(); $this->container['OCP\\App\\IAppManager'] @@ -779,6 +955,8 @@ class UsersControllerTest extends \Test\TestCase { } public function testRestoreNotPossibleWithoutAdminRestore() { + $this->container['IsAdmin'] = true; + list($user, $expectedResult) = $this->mockUser(); $this->container['OCP\\App\\IAppManager'] @@ -795,6 +973,8 @@ class UsersControllerTest extends \Test\TestCase { } public function testRestoreNotPossibleWithoutUserRestore() { + $this->container['IsAdmin'] = true; + list($user, $expectedResult) = $this->mockUser(); $this->container['OCP\\App\\IAppManager']