From 7666497c0ada87c5df67a793e22152d478f2740a Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Wed, 14 Mar 2018 01:12:42 +0300 Subject: [PATCH] Introduce UserSearch utils --- apps/files_sharing/lib/Capabilities.php | 17 +- apps/files_sharing/tests/CapabilitiesTest.php | 20 +- core/js/sharedialogview.js | 2 +- core/js/tests/specs/sharedialogviewSpec.js | 7 + lib/private/Group/Manager.php | 45 ++- lib/private/Server.php | 11 +- lib/private/User/Manager.php | 55 +++- lib/public/Util/UserSearch.php | 68 ++++ .../features/apiSharees/sharees.feature | 2 +- .../shareAutocompletion.feature | 28 +- tests/lib/Files/Config/UserMountCacheTest.php | 14 +- tests/lib/Group/ManagerTest.php | 305 ++++++------------ tests/lib/User/ManagerTest.php | 16 +- 13 files changed, 324 insertions(+), 266 deletions(-) create mode 100644 lib/public/Util/UserSearch.php diff --git a/apps/files_sharing/lib/Capabilities.php b/apps/files_sharing/lib/Capabilities.php index d2a336ca4e10..f46393f07399 100644 --- a/apps/files_sharing/lib/Capabilities.php +++ b/apps/files_sharing/lib/Capabilities.php @@ -22,6 +22,7 @@ use OCP\Capabilities\ICapability; use OCP\IConfig; +use OCP\Util\UserSearch; /** * Class Capabilities @@ -33,8 +34,20 @@ class Capabilities implements ICapability { /** @var IConfig */ private $config; - public function __construct(IConfig $config) { + /** + * @var UserSearch + */ + private $userSearch; + + /** + * Capabilities constructor. + * + * @param IConfig $config + * @param UserSearch $userSearch + */ + public function __construct(IConfig $config, UserSearch $userSearch) { $this->config = $config; + $this->userSearch = $userSearch; } /** @@ -99,6 +112,8 @@ public function getCapabilities() { 'incoming' => $this->config->getAppValue('files_sharing', 'incoming_server2server_share_enabled', 'yes') === 'yes' ]; + $res['search_min_length'] = $this->userSearch->getSearchMinLength(); + return [ 'files_sharing' => $res, ]; diff --git a/apps/files_sharing/tests/CapabilitiesTest.php b/apps/files_sharing/tests/CapabilitiesTest.php index 4104ea4ff1ba..b50d92e6854e 100644 --- a/apps/files_sharing/tests/CapabilitiesTest.php +++ b/apps/files_sharing/tests/CapabilitiesTest.php @@ -31,6 +31,24 @@ */ class CapabilitiesTest extends \Test\TestCase { + /** + * @var \OCP\Util\UserSearch + */ + protected $userSearch; + + /** + * + */ + protected function setUp() { + parent::setUp(); + $this->userSearch = $this->getMockBuilder(\OCP\Util\UserSearch::class) + ->disableOriginalConstructor() + ->getMock(); + $this->userSearch->expects($this->any()) + ->method('getSearchMinLength') + ->willReturn(1); + } + /** * Test for the general part in each return statement and assert. * Strip off the general part on the way. @@ -54,7 +72,7 @@ private function getFilesSharingPart(array $data) { private function getResults(array $map) { $stub = $this->getMockBuilder('\OCP\IConfig')->disableOriginalConstructor()->getMock(); $stub->method('getAppValue')->will($this->returnValueMap($map)); - $cap = new Capabilities($stub); + $cap = new Capabilities($stub, $this->userSearch); $result = $this->getFilesSharingPart($cap->getCapabilities()); return $result; } diff --git a/core/js/sharedialogview.js b/core/js/sharedialogview.js index 7082e4a811af..f829d9510ad3 100644 --- a/core/js/sharedialogview.js +++ b/core/js/sharedialogview.js @@ -404,7 +404,7 @@ var $shareField = this.$el.find('.shareWithField'); if ($shareField.length) { $shareField.autocomplete({ - minLength: 1, + minLength: OC.getCapabilities().files_sharing.search_min_length, delay: 750, focus: function(event) { event.preventDefault(); diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index 3a4368bcf528..77c8c7263cbe 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -105,8 +105,15 @@ describe('OC.Share.ShareDialogView', function() { oldCurrentUser = OC.currentUser; OC.currentUser = 'user0'; + capsSpec = sinon.stub(OC, 'getCapabilities'); + capsSpec.returns({ + 'files_sharing': { + 'search_min_length': 4 + } + }); }); afterEach(function() { + capsSpec.restore(); OC.currentUser = oldCurrentUser; /* jshint camelcase:false */ oc_appconfig.core = oldAppConfig; diff --git a/lib/private/Group/Manager.php b/lib/private/Group/Manager.php index bad079939030..047d00e8eb11 100644 --- a/lib/private/Group/Manager.php +++ b/lib/private/Group/Manager.php @@ -36,8 +36,10 @@ namespace OC\Group; use OC\Hooks\PublicEmitter; +use OC\User\Manager as UserManager; use OCP\GroupInterface; use OCP\IGroupManager; +use OCP\Util\UserSearch; /** * Class Manager @@ -61,10 +63,15 @@ class Manager extends PublicEmitter implements IGroupManager { private $backends = []; /** - * @var \OC\User\Manager $userManager + * @var UserManager $userManager */ private $userManager; + /** + * @var UserSearch $userSearch + */ + private $userSearch; + /** * @var \OC\Group\Group[] */ @@ -80,9 +87,11 @@ class Manager extends PublicEmitter implements IGroupManager { /** * @param \OC\User\Manager $userManager + * @param UserSearch $userSearch */ - public function __construct(\OC\User\Manager $userManager) { + public function __construct(UserManager $userManager, UserSearch $userSearch) { $this->userManager = $userManager; + $this->userSearch = $userSearch; $cachedGroups = & $this->cachedGroups; $cachedUserGroups = & $this->cachedUserGroups; $this->listen('\OC\Group', 'postDelete', function ($group) use (&$cachedGroups, &$cachedUserGroups) { @@ -221,22 +230,24 @@ public function createGroup($gid) { */ public function search($search, $limit = null, $offset = null, $scope = null) { $groups = []; - foreach ($this->backends as $backend) { - if (!$backend->isVisibleForScope($scope)) { - // skip backend - continue; - } - $groupIds = $backend->getGroups($search, $limit, $offset); - foreach ($groupIds as $groupId) { - $aGroup = $this->get($groupId); - if (!is_null($aGroup)) { - $groups[$groupId] = $aGroup; - } else { - \OC::$server->getLogger()->debug('Group "' . $groupId . '" was returned by search but not found through direct access', array('app' => 'core')); + if ($this->userSearch->isSearchable($search)) { + foreach ($this->backends as $backend) { + if (!$backend->isVisibleForScope($scope)) { + // skip backend + continue; + } + $groupIds = $backend->getGroups($search, $limit, $offset); + foreach ($groupIds as $groupId) { + $aGroup = $this->get($groupId); + if (!is_null($aGroup)) { + $groups[$groupId] = $aGroup; + } else { + \OC::$server->getLogger()->debug('Group "' . $groupId . '" was returned by search but not found through direct access', array('app' => 'core')); + } + } + if (!is_null($limit) and $limit <= 0) { + return array_values($groups); } - } - if (!is_null($limit) and $limit <= 0) { - return array_values($groups); } } return array_values($groups); diff --git a/lib/private/Server.php b/lib/private/Server.php index edbc61af1513..79770166fd9d 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -97,6 +97,7 @@ use OCP\IUser; use OCP\Security\IContentSecurityPolicyManager; use OCP\Theme\IThemeService; +use OCP\Util\UserSearch; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use OC\Files\External\StoragesBackendService; @@ -248,11 +249,19 @@ public function __construct($webRoot, \OC\Config $config) { $c->getConfig(), $c->getLogger(), $c->getAccountMapper() + ), + new UserSearch( + $c->getConfig() ) ); }); $this->registerService('GroupManager', function (Server $c) { - $groupManager = new \OC\Group\Manager($this->getUserManager()); + $groupManager = new \OC\Group\Manager( + $this->getUserManager(), + new UserSearch( + $c->getConfig() + ) + ); $groupManager->listen('\OC\Group', 'preCreate', function ($gid) { \OC_Hook::emit('OC_Group', 'pre_createGroup', ['run' => true, 'gid' => $gid]); }); diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 255cc7729159..dedc33808fdd 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -45,6 +45,7 @@ use OCP\User\IProvidesEMailBackend; use OCP\User\IProvidesQuotaBackend; use OCP\UserInterface; +use OCP\Util\UserSearch; use Symfony\Component\EventDispatcher\GenericEvent; /** @@ -81,23 +82,39 @@ class Manager extends PublicEmitter implements IUserManager { /** @var SyncService */ private $syncService; + /** + * @var UserSearch + */ + private $userSearch; + /** * @param IConfig $config * @param ILogger $logger * @param AccountMapper $accountMapper * @param SyncService $syncService + * @param UserSearch $userSearch */ - public function __construct(IConfig $config, ILogger $logger, AccountMapper $accountMapper, SyncService $syncService) { + public function __construct( + IConfig $config, + ILogger $logger, + AccountMapper $accountMapper, + SyncService $syncService, + UserSearch $userSearch + ) { $this->config = $config; $this->logger = $logger; $this->accountMapper = $accountMapper; $this->cachedUsers = new CappedMemoryCache(); $this->syncService = $syncService; + $this->userSearch = $userSearch; $cachedUsers = &$this->cachedUsers; - $this->listen('\OC\User', 'postDelete', function ($user) use (&$cachedUsers) { - /** @var \OC\User\User $user */ - unset($cachedUsers[$user->getUID()]); - }); + $this->listen( + '\OC\User', 'postDelete', + function ($user) use (&$cachedUsers) { + /** @var \OC\User\User $user */ + unset($cachedUsers[$user->getUID()]); + } + ); } /** @@ -248,9 +265,11 @@ public function checkPassword($loginName, $password) { public function search($pattern, $limit = null, $offset = null) { $accounts = $this->accountMapper->search('user_id', $pattern, $limit, $offset); $users = []; - foreach ($accounts as $account) { - $user = $this->getUserObject($account); - $users[$user->getUID()] = $user; + if ($this->userSearch->isSearchable($pattern)) { + foreach ($accounts as $account) { + $user = $this->getUserObject($account); + $users[$user->getUID()] = $user; + } } return $users; @@ -267,9 +286,11 @@ public function search($pattern, $limit = null, $offset = null) { public function find($pattern, $limit = null, $offset = null) { $accounts = $this->accountMapper->find($pattern, $limit, $offset); $users = []; - foreach ($accounts as $account) { - $user = $this->getUserObject($account); - $users[$user->getUID()] = $user; + if ($this->userSearch->isSearchable($pattern)) { + foreach ($accounts as $account) { + $user = $this->getUserObject($account); + $users[$user->getUID()] = $user; + } } return $users; } @@ -283,10 +304,14 @@ public function find($pattern, $limit = null, $offset = null) { * @return \OC\User\User[] */ public function searchDisplayName($pattern, $limit = null, $offset = null) { - $accounts = $this->accountMapper->search('display_name', $pattern, $limit, $offset); - return array_map(function(Account $account) { - return $this->getUserObject($account); - }, $accounts); + if ($this->userSearch->isSearchable($pattern)) { + $accounts = $this->accountMapper->search('display_name', $pattern, $limit, $offset); + return array_map(function(Account $account) { + return $this->getUserObject($account); + }, $accounts); + + } + return []; } /** diff --git a/lib/public/Util/UserSearch.php b/lib/public/Util/UserSearch.php new file mode 100644 index 000000000000..da4973eb446c --- /dev/null +++ b/lib/public/Util/UserSearch.php @@ -0,0 +1,68 @@ + + * + * @copyright Copyright (c) 2018, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +/** + * Public interface of ownCloud for apps to use. + * App Class + * + */ + + +namespace OCP\Util; +use OCP\IConfig; + +/** + * This class provides utility functions for searching users and groups + * + * @since 10.0.8 + */ + +class UserSearch { + + protected $config; + + /** + * UserSearch constructor. + * + * @param IConfig $config + */ + public function __construct(IConfig $config) { + $this->config = $config; + } + + /** + * @param string $pattern + * @return bool + */ + public function isSearchable($pattern) { + $trimmed = trim($pattern); + return $trimmed === '' || strlen($trimmed) >= $this->getSearchMinLength(); + } + + /** + * Get minimal allowed size to search users + * + * @return mixed + */ + public function getSearchMinLength() { + return $this->config->getSystemValue('user.search_min_length', 4); + } +} diff --git a/tests/acceptance/features/apiSharees/sharees.feature b/tests/acceptance/features/apiSharees/sharees.feature index 516cc2f92fa0..18cf9fbd6e90 100644 --- a/tests/acceptance/features/apiSharees/sharees.feature +++ b/tests/acceptance/features/apiSharees/sharees.feature @@ -296,7 +296,7 @@ Feature: sharees And user "Another" has been added to group "ShareeGroup2" And parameter "shareapi_share_dialog_user_enumeration_group_members" of app "core" has been set to "yes" When user "test" gets the sharees using the API with parameters - | search | ano | + | search | anot | | itemType | file | Then the OCS status code should be "100" And the HTTP status code should be "200" diff --git a/tests/acceptance/features/webUISharingInternalUsers/shareAutocompletion.feature b/tests/acceptance/features/webUISharingInternalUsers/shareAutocompletion.feature index 62160813db85..8dbdadbc1490 100644 --- a/tests/acceptance/features/webUISharingInternalUsers/shareAutocompletion.feature +++ b/tests/acceptance/features/webUISharingInternalUsers/shareAutocompletion.feature @@ -14,10 +14,10 @@ So that I can efficiently share my files with other users or groups | regularuser | 1234 | User Regular | regularuser@oc.com.np | And these groups have been created: |groupname| - |grp1 | - |grp2 | - |grp3 | - |grpuser | + |group1 | + |group2 | + |group3 | + |groupuser | And the user has browsed to the login page And the user has logged in with username "regularuser" and password "1234" using the webUI And the user has browsed to the files page @@ -31,8 +31,8 @@ So that I can efficiently share my files with other users or groups Scenario: autocompletion of regular existing groups Given the user has opened the share dialog for the folder "simple-folder" - When the user types "grp" in the share-with-field - Then all users and groups that contain the string "grp" in their name should be listed in the autocomplete list on the webUI + When the user types "grou" in the share-with-field + Then all users and groups that contain the string "grou" in their name should be listed in the autocomplete list on the webUI And the users own name should not be listed in the autocomplete list on the webUI Scenario: autocompletion for a pattern that does not match any user or group @@ -50,7 +50,7 @@ So that I can efficiently share my files with other users or groups And the users own name should not be listed in the autocomplete list on the webUI @skipOnLDAP @user_ldap#175 - Scenario: autocompletion of a pattern that matches regular existing users but also a user whith whom the item is already shared (file) + Scenario: autocompletion of a pattern that matches regular existing users but also a user with whom the item is already shared (file) Given the user has shared the file "data.zip" with the user "User Grp" using the webUI And the user has opened the share dialog for the file "data.zip" When the user types "user" in the share-with-field @@ -58,15 +58,15 @@ So that I can efficiently share my files with other users or groups And the users own name should not be listed in the autocomplete list on the webUI Scenario: autocompletion of a pattern that matches regular existing groups but also a group with whom the item is already shared (folder) - Given the user shares the folder "simple-folder" with the group "grp1" using the webUI + Given the user shares the folder "simple-folder" with the group "group1" using the webUI And the user has opened the share dialog for the folder "simple-folder" - When the user types "grp" in the share-with-field - Then all users and groups that contain the string "grp" in their name should be listed in the autocomplete list on the webUI except group "grp1" + When the user types "grou" in the share-with-field + Then all users and groups that contain the string "grou" in their name should be listed in the autocomplete list on the webUI except group "group1" And the users own name should not be listed in the autocomplete list on the webUI - Scenario: autocompletion of a pattern that matches regular existing groups but also a group whith whom the item is already shared (file) - Given the user shares the file "data.zip" with the group "grpuser" using the webUI + Scenario: autocompletion of a pattern that matches regular existing groups but also a group with whom the item is already shared (file) + Given the user shares the file "data.zip" with the group "groupuser" using the webUI And the user has opened the share dialog for the file "data.zip" - When the user types "grp" in the share-with-field - Then all users and groups that contain the string "grp" in their name should be listed in the autocomplete list on the webUI except group "grpuser" + When the user types "grou" in the share-with-field + Then all users and groups that contain the string "grou" in their name should be listed in the autocomplete list on the webUI except group "groupuser" And the users own name should not be listed in the autocomplete list on the webUI diff --git a/tests/lib/Files/Config/UserMountCacheTest.php b/tests/lib/Files/Config/UserMountCacheTest.php index 41e3a7893dfc..031b6932b33a 100644 --- a/tests/lib/Files/Config/UserMountCacheTest.php +++ b/tests/lib/Files/Config/UserMountCacheTest.php @@ -41,11 +41,23 @@ class UserMountCacheTest extends TestCase { */ private $cache; + /** + * @var \OCP\Util\UserSearch + */ + protected $userSearch; + private $fileIds = []; public function setUp() { $this->fileIds = []; $this->connection = \OC::$server->getDatabaseConnection(); + $this->userSearch = $this->getMockBuilder(\OCP\Util\UserSearch::class) + ->disableOriginalConstructor() + ->getMock(); + $this->userSearch->expects($this->any()) + ->method('isSearchable') + ->willReturn(true); + /** @var IConfig $config */ $config = $this->createMock(IConfig::class); /** @var AccountMapper | \PHPUnit_Framework_MockObject_MockObject $accountMapper */ @@ -70,7 +82,7 @@ public function setUp() { $log = $this->createMock(Log::class); /** @var SyncService $syncService */ $syncService = $this->createMock(SyncService::class); - $this->userManager = new Manager($config, $log, $accountMapper, $syncService); + $this->userManager = new Manager($config, $log, $accountMapper, $syncService, $this->userSearch); $this->cache = new UserMountCache($this->connection, $this->userManager, $log); // hookup listener diff --git a/tests/lib/Group/ManagerTest.php b/tests/lib/Group/ManagerTest.php index 9bf7cb4e8c6b..fa9c71d71e42 100644 --- a/tests/lib/Group/ManagerTest.php +++ b/tests/lib/Group/ManagerTest.php @@ -26,8 +26,32 @@ use OC\User\Manager; use OCP\GroupInterface; use OCP\IUser; +use OCP\Util\UserSearch; class ManagerTest extends \Test\TestCase { + + /** + * @var \OC\User\Manager + */ + protected $userManager; + + /** + * @var \OCP\Util\UserSearch + */ + protected $userSearch; + + protected function setUp() { + parent::setUp(); + $this->userSearch = $this->getMockBuilder(UserSearch::class) + ->disableOriginalConstructor() + ->getMock(); + $this->userSearch->expects($this->any()) + ->method('isSearchable') + ->willReturn(true); + + $this->userManager = $this->createMock(Manager::class); + } + private function getTestUser($userId) { $mockUser = $this->createMock(IUser::class); $mockUser->expects($this->any()) @@ -93,11 +117,8 @@ public function testGet() { ->with('group1') ->will($this->returnValue(true)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager); + + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $group = $manager->get('group1'); @@ -106,11 +127,7 @@ public function testGet() { } public function testGetNoBackend() { - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $this->assertNull($manager->get('group1')); } @@ -125,11 +142,8 @@ public function testGetNotExists() { ->with('group1') ->will($this->returnValue(false)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager); + + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $this->assertNull($manager->get('group1')); @@ -139,11 +153,7 @@ public function testGetDeleted() { $backend = new \Test\Util\Group\Dummy(); $backend->createGroup('group1'); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $group = $manager->get('group1'); @@ -170,11 +180,7 @@ public function testGetMultipleBackends() { ->with('group1') ->will($this->returnValue(true)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend1); $manager->addBackend($backend2); @@ -201,11 +207,7 @@ public function testCreate() { $backendGroupCreated = true; }));; - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $group = $manager->createGroup('group1'); @@ -224,11 +226,7 @@ public function testCreateExists() { $backend->expects($this->never()) ->method('createGroup'); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $group = $manager->createGroup('group1'); @@ -249,11 +247,7 @@ public function testSearch() { ->with('group1') ->will($this->returnValue(true)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $groups = $manager->search('1'); @@ -287,11 +281,7 @@ public function testSearchMultipleBackends() { ->method('groupExists') ->will($this->returnValue(true)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend1); $manager->addBackend($backend2); @@ -328,11 +318,7 @@ public function testSearchMultipleBackendsLimitAndOffset() { ->method('groupExists') ->will($this->returnValue(true)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend1); $manager->addBackend($backend2); @@ -361,12 +347,7 @@ public function testSearchResultExistsButGroupDoesNot() { ->method('isVisibleForScope') ->will($this->returnValue(true)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $groups = $manager->search('1'); @@ -401,11 +382,7 @@ public function testSearchBackendsForScope() { ->method('groupExists') ->will($this->returnValue(true)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend1); $manager->addBackend($backend2); @@ -438,12 +415,8 @@ public function testGetUserGroups() { ->with('group1') ->will($this->returnValue(true)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); $userBackend = $this->createMock(\OC_User_Backend::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $groups = $manager->getUserGroups($this->getTestUser('user1')); @@ -492,12 +465,8 @@ public function testGetUserGroupsWithDeletedGroup() { ->with('group1') ->will($this->returnValue(false)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); $userBackend = $this->createMock(\OC_User_Backend::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); /** @var \OC\User\User $user */ @@ -525,12 +494,8 @@ public function testGetUserGroupsWithScope() { ->with('group1') ->will($this->returnValue(true)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); $userBackend = $this->createMock(\OC_User_Backend::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $groups = $manager->getUserGroups($this->getTestUser('user1')); @@ -555,12 +520,8 @@ public function testIsInGroup() { ->method('groupExists') ->will($this->returnValue(true)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); $userBackend = $this->createMock(\OC_User_Backend::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $this->assertTrue($manager->isInGroup('user1', 'group1')); @@ -583,15 +544,11 @@ public function testInGroup() { * @var IUser */ $user = $this->createMock(IUser::class); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $userManager->expects($this->once()) + $this->userManager->expects($this->once()) ->method('get') ->with('user1') ->will($this->returnValue($user)); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $this->assertTrue($manager->inGroup('user1', 'group1')); @@ -614,15 +571,11 @@ public function testNotInGroup() { * @var IUser */ $user = $this->createMock(IUser::class); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $userManager->expects($this->once()) + $this->userManager->expects($this->once()) ->method('get') ->with('user1') ->will($this->returnValue($user)); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $this->assertFalse($manager->inGroup('user1', 'group1')); @@ -641,12 +594,8 @@ public function testIsAdmin() { ->method('groupExists') ->will($this->returnValue(true)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); $userBackend = $this->createMock(\OC_User_Backend::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $this->assertTrue($manager->isAdmin('user1')); @@ -665,12 +614,8 @@ public function testNotAdmin() { ->method('groupExists') ->will($this->returnValue(true)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); $userBackend = $this->createMock(\OC_User_Backend::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $this->assertFalse($manager->isAdmin('user1')); @@ -701,12 +646,8 @@ public function testGetUserGroupsMultipleBackends() { ->method('groupExists') ->will($this->returnValue(true)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); $userBackend = $this->createMock(\OC_User_Backend::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend1); $manager->addBackend($backend2); @@ -741,13 +682,9 @@ public function testDisplayNamesInGroupWithOneUserBackend() { } })); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); $userBackend = $this->createMock(\OC_User_Backend::class); - $userManager->expects($this->any()) + $this->userManager->expects($this->any()) ->method('searchDisplayName') ->with('user3') ->will($this->returnCallback(function($search, $limit, $offset) use ($userBackend) { @@ -758,7 +695,7 @@ public function testDisplayNamesInGroupWithOneUserBackend() { } })); - $userManager->expects($this->any()) + $this->userManager->expects($this->any()) ->method('get') ->will($this->returnCallback(function($uid) use ($userBackend) { switch($uid) { @@ -771,7 +708,7 @@ public function testDisplayNamesInGroupWithOneUserBackend() { } })); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $users = $manager->displayNamesInGroup('testgroup', 'user3'); @@ -806,13 +743,9 @@ public function testDisplayNamesInGroupWithOneUserBackendWithLimitSpecified() { } })); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); $userBackend = $this->createMock(\OC_User_Backend::class); - $userManager->expects($this->any()) + $this->userManager->expects($this->any()) ->method('searchDisplayName') ->with('user3') ->will($this->returnCallback(function($search, $limit, $offset) use ($userBackend) { @@ -823,7 +756,7 @@ public function testDisplayNamesInGroupWithOneUserBackendWithLimitSpecified() { } })); - $userManager->expects($this->any()) + $this->userManager->expects($this->any()) ->method('get') ->will($this->returnCallback(function($uid) use ($userBackend) { switch($uid) { @@ -837,7 +770,7 @@ public function testDisplayNamesInGroupWithOneUserBackendWithLimitSpecified() { } })); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $users = $manager->displayNamesInGroup('testgroup', 'user3', 1); @@ -873,13 +806,9 @@ public function testDisplayNamesInGroupWithOneUserBackendWithLimitAndOffsetSpeci } })); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); $userBackend = $this->createMock(\OC_User_Backend::class); - $userManager->expects($this->any()) + $this->userManager->expects($this->any()) ->method('searchDisplayName') ->with('user3') ->will($this->returnCallback(function($search, $limit, $offset) use ($userBackend) { @@ -893,7 +822,7 @@ public function testDisplayNamesInGroupWithOneUserBackendWithLimitAndOffsetSpeci } })); - $userManager->expects($this->any()) + $this->userManager->expects($this->any()) ->method('get') ->will($this->returnCallback(function($uid) use ($userBackend) { switch($uid) { @@ -907,7 +836,7 @@ public function testDisplayNamesInGroupWithOneUserBackendWithLimitAndOffsetSpeci } })); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $users = $manager->displayNamesInGroup('testgroup', 'user3', 1, 1); @@ -934,13 +863,9 @@ public function testDisplayNamesInGroupWithOneUserBackendAndSearchEmpty() { ->with('testgroup', '', -1, 0) ->will($this->returnValue(['user2', 'user33'])); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); $userBackend = $this->createMock(\OC_User_Backend::class); - $userManager->expects($this->any()) + $this->userManager->expects($this->any()) ->method('get') ->will($this->returnCallback(function($uid) use ($userBackend) { switch($uid) { @@ -953,7 +878,7 @@ public function testDisplayNamesInGroupWithOneUserBackendAndSearchEmpty() { } })); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $users = $manager->displayNamesInGroup('testgroup', ''); @@ -978,13 +903,9 @@ public function testDisplayNamesInGroupWithOneUserBackendAndSearchEmptyAndLimitS ->method('usersInGroup') ->with('testgroup', '', 1, 0) ->will($this->returnValue(['user2'])); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $userBackend = $this->createMock(\OC_User_Backend::class); - $userManager->expects($this->any()) + $userBackend = $this->createMock(\OC_User_Backend::class); + $this->userManager->expects($this->any()) ->method('get') ->will($this->returnCallback(function($uid) use ($userBackend) { switch($uid) { @@ -997,7 +918,7 @@ public function testDisplayNamesInGroupWithOneUserBackendAndSearchEmptyAndLimitS } })); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $users = $manager->displayNamesInGroup('testgroup', '', 1); @@ -1023,13 +944,9 @@ public function testDisplayNamesInGroupWithOneUserBackendAndSearchEmptyAndLimitA ->with('testgroup', '', 1, 1) ->will($this->returnValue(['user33'])); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $userBackend = $this->createMock(\OC_User_Backend::class); - $userManager->expects($this->any()) + $userBackend = $this->createMock(\OC_User_Backend::class); + $this->userManager->expects($this->any()) ->method('get') ->will($this->returnCallback(function($uid) use ($userBackend) { switch($uid) { @@ -1042,7 +959,7 @@ public function testDisplayNamesInGroupWithOneUserBackendAndSearchEmptyAndLimitA } })); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $users = $manager->displayNamesInGroup('testgroup', '', 1, 1); @@ -1070,11 +987,8 @@ public function testGetUserGroupsWithAddUser() { ->with('group1') ->will($this->returnValue(true)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager); + + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); // prime cache @@ -1117,11 +1031,8 @@ public function testGetUserGroupsWithRemoveUser() { ->method('removeFromGroup') ->will($this->returnValue(true)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager); + + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); // prime cache @@ -1138,7 +1049,7 @@ public function testGetUserGroupsWithRemoveUser() { // check result $groups = $manager->getUserGroups($user1); - $this->assertEquals([], $groups); + $this->assertEquals($expectedGroups, $groups); } public function testGetUserIdGroups() { @@ -1151,11 +1062,8 @@ public function testGetUserIdGroups() { ->with('user1') ->will($this->returnValue(null)); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager); + + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $groups = $manager->getUserIdGroups('user1'); @@ -1181,11 +1089,7 @@ public function testGroupDisplayName() { ['group2', ['gid' => 'group2']], ])); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); // group with display name @@ -1224,13 +1128,9 @@ public function testFindUsersInGroupWithOneUserBackend() { } })); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); $userBackend = $this->createMock(\OC_User_Backend::class); - $userManager->expects($this->any()) + $this->userManager->expects($this->any()) ->method('find') ->with('user3') ->will($this->returnCallback(function($search, $limit, $offset) use ($userBackend) { @@ -1241,7 +1141,7 @@ public function testFindUsersInGroupWithOneUserBackend() { } })); - $userManager->expects($this->any()) + $this->userManager->expects($this->any()) ->method('get') ->will($this->returnCallback(function($uid) use ($userBackend) { switch($uid) { @@ -1254,7 +1154,7 @@ public function testFindUsersInGroupWithOneUserBackend() { } })); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $users = $manager->findUsersInGroup('testgroup', 'user3'); @@ -1289,13 +1189,9 @@ public function testFindUsersInGroupWithOneUserBackendWithLimitSpecified() { } })); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); $userBackend = $this->createMock(\OC_User_Backend::class); - $userManager->expects($this->any()) + $this->userManager->expects($this->any()) ->method('find') ->with('user3') ->will($this->returnCallback(function($search, $limit, $offset) use ($userBackend) { @@ -1306,7 +1202,7 @@ public function testFindUsersInGroupWithOneUserBackendWithLimitSpecified() { } })); - $userManager->expects($this->any()) + $this->userManager->expects($this->any()) ->method('get') ->will($this->returnCallback(function($uid) use ($userBackend) { switch($uid) { @@ -1320,7 +1216,7 @@ public function testFindUsersInGroupWithOneUserBackendWithLimitSpecified() { } })); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $users = $manager->findUsersInGroup('testgroup', 'user3', 1); @@ -1356,13 +1252,9 @@ public function testFindUsersInGroupWithOneUserBackendWithLimitAndOffsetSpecifie } })); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $userBackend = $this->createMock(\OC_User_Backend::class); - $userManager->expects($this->any()) + $userBackend = $this->createMock(\OC_User_Backend::class); + $this->userManager->expects($this->any()) ->method('find') ->with('user3') ->will($this->returnCallback(function($search, $limit, $offset) use ($userBackend) { @@ -1376,7 +1268,7 @@ public function testFindUsersInGroupWithOneUserBackendWithLimitAndOffsetSpecifie } })); - $userManager->expects($this->any()) + $this->userManager->expects($this->any()) ->method('get') ->will($this->returnCallback(function($uid) use ($userBackend) { switch($uid) { @@ -1390,7 +1282,7 @@ public function testFindUsersInGroupWithOneUserBackendWithLimitAndOffsetSpecifie } })); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $users = $manager->findUsersInGroup('testgroup', 'user3', 1, 1); @@ -1417,13 +1309,8 @@ public function testFindUsersInGroupWithOneUserBackendAndSearchEmpty() { ->with('testgroup', '', -1, 0) ->will($this->returnValue(['user2', 'user33'])); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); $userBackend = $this->createMock(\OC_User_Backend::class); - - $userManager->expects($this->any()) + $this->userManager->expects($this->any()) ->method('get') ->will($this->returnCallback(function($uid) use ($userBackend) { switch($uid) { @@ -1436,7 +1323,7 @@ public function testFindUsersInGroupWithOneUserBackendAndSearchEmpty() { } })); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $users = $manager->findUsersInGroup('testgroup', ''); @@ -1461,13 +1348,9 @@ public function testFindUsersInGroupWithOneUserBackendAndSearchEmptyAndLimitSpec ->method('usersInGroup') ->with('testgroup', '', 1, 0) ->will($this->returnValue(['user2'])); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $userBackend = $this->createMock(\OC_User_Backend::class); - $userManager->expects($this->any()) + $userBackend = $this->createMock(\OC_User_Backend::class); + $this->userManager->expects($this->any()) ->method('get') ->will($this->returnCallback(function($uid) use ($userBackend) { switch($uid) { @@ -1480,7 +1363,7 @@ public function testFindUsersInGroupWithOneUserBackendAndSearchEmptyAndLimitSpec } })); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $users = $manager->findUsersInGroup('testgroup', '', 1); @@ -1506,13 +1389,9 @@ public function testFindUsersInGroupWithOneUserBackendAndSearchEmptyAndLimitAndO ->with('testgroup', '', 1, 1) ->will($this->returnValue(['user33'])); - /** - * @var \OC\User\Manager $userManager - */ - $userManager = $this->createMock(Manager::class); - $userBackend = $this->createMock(\OC_User_Backend::class); - $userManager->expects($this->any()) + $userBackend = $this->createMock(\OC_User_Backend::class); + $this->userManager->expects($this->any()) ->method('get') ->will($this->returnCallback(function($uid) use ($userBackend) { switch($uid) { @@ -1525,7 +1404,7 @@ public function testFindUsersInGroupWithOneUserBackendAndSearchEmptyAndLimitAndO } })); - $manager = new \OC\Group\Manager($userManager); + $manager = new \OC\Group\Manager($this->userManager, $this->userSearch); $manager->addBackend($backend); $users = $manager->findUsersInGroup('testgroup', '', 1, 1); diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php index b2da9ae8fe8d..e00597baa5a4 100644 --- a/tests/lib/User/ManagerTest.php +++ b/tests/lib/User/ManagerTest.php @@ -38,6 +38,11 @@ class ManagerTest extends TestCase { /** @var SyncService | \PHPUnit_Framework_MockObject_MockObject */ private $syncService; + /** + * @var \OCP\Util\UserSearch + */ + protected $userSearch; + public function setUp() { parent::setUp(); @@ -47,7 +52,16 @@ public function setUp() { $logger = $this->createMock(ILogger::class); $this->accountMapper = $this->createMock(AccountMapper::class); $this->syncService = $this->createMock(SyncService::class); - $this->manager = new \OC\User\Manager($config, $logger, $this->accountMapper, $this->syncService); + + $this->userSearch = $this->getMockBuilder(\OCP\Util\UserSearch::class) + ->disableOriginalConstructor() + ->getMock(); + $this->userSearch->expects($this->any()) + ->method('isSearchable') + ->willReturn(true); + $this->manager = new \OC\User\Manager( + $config, $logger, $this->accountMapper, $this->syncService, $this->userSearch + ); } public function testGetBackends() {