Skip to content

Commit

Permalink
Merge pull request #30994 from owncloud/stable10-fix-30313
Browse files Browse the repository at this point in the history
[Stable10] Introduce UserSearch utils
  • Loading branch information
Vincent Petry authored Apr 3, 2018
2 parents 3fd2236 + 541f639 commit d912c88
Show file tree
Hide file tree
Showing 13 changed files with 324 additions and 254 deletions.
17 changes: 16 additions & 1 deletion apps/files_sharing/lib/Capabilities.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

use OCP\Capabilities\ICapability;
use OCP\IConfig;
use OCP\Util\UserSearch;

/**
* Class Capabilities
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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,
];
Expand Down
20 changes: 19 additions & 1 deletion apps/files_sharing/tests/CapabilitiesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion core/js/sharedialogview.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
7 changes: 7 additions & 0 deletions core/js/tests/specs/sharedialogviewSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
45 changes: 28 additions & 17 deletions lib/private/Group/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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[]
*/
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
11 changes: 10 additions & 1 deletion lib/private/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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]);
});
Expand Down
55 changes: 40 additions & 15 deletions lib/private/User/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
use OCP\User\IProvidesEMailBackend;
use OCP\User\IProvidesQuotaBackend;
use OCP\UserInterface;
use OCP\Util\UserSearch;
use Symfony\Component\EventDispatcher\GenericEvent;

/**
Expand Down Expand Up @@ -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()]);
}
);
}

/**
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand All @@ -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 [];
}

/**
Expand Down
71 changes: 71 additions & 0 deletions lib/public/Util/UserSearch.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php
/**
* @author Viktar Dubiniuk <[email protected]>
*
* @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 <http://www.gnu.org/licenses/>
*
*/

/**
* 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
* @since 10.0.8
*/
public function __construct(IConfig $config) {
$this->config = $config;
}

/**
* @param string $pattern
* @return bool
* @since 10.0.8
*/
public function isSearchable($pattern) {
$trimmed = trim($pattern);
return $trimmed === '' || strlen($trimmed) >= $this->getSearchMinLength();
}

/**
* Get minimal allowed size to search users
*
* @return mixed
* @since 10.0.8
*/
public function getSearchMinLength() {
return $this->config->getSystemValue('user.search_min_length', 4);
}
}
2 changes: 1 addition & 1 deletion tests/acceptance/features/apiSharees/sharees.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Loading

0 comments on commit d912c88

Please sign in to comment.