Skip to content

Commit

Permalink
Search limit draft
Browse files Browse the repository at this point in the history
  • Loading branch information
@VicDeo
VicDeo committed Mar 13, 2018
1 parent 33c57ac commit 2f08494
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 26 deletions.
2 changes: 2 additions & 0 deletions apps/files_sharing/lib/Capabilities.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ public function getCapabilities() {
'incoming' => $this->config->getAppValue('files_sharing', 'incoming_server2server_share_enabled', 'yes') === 'yes'
];

$res['search_min_length'] = \OC::$server->getUserManager()->getSearchMinLength();

return [
'files_sharing' => $res,
];
Expand Down
2 changes: 1 addition & 1 deletion core/js/sharedialogview.js
View file
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
32 changes: 17 additions & 15 deletions lib/private/Group/Manager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,22 +221,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->userManager->isSearcheable($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
41 changes: 31 additions & 10 deletions lib/private/User/Manager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,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->isSearcheable($pattern)) {
foreach ($accounts as $account) {
$user = $this->getUserObject($account);
$users[$user->getUID()] = $user;
}
}

return $users;
Expand All @@ -267,9 +269,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->isSearcheable($pattern)) {
foreach ($accounts as $account) {
$user = $this->getUserObject($account);
$users[$user->getUID()] = $user;
}
}
return $users;
}
Expand All @@ -283,10 +287,27 @@ 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->isSearcheable($pattern)) {
$accounts = $this->accountMapper->search('display_name', $pattern, $limit, $offset);
return array_map(function(Account $account) {
return $this->getUserObject($account);
}, $accounts);

}
return [];
}

/**
* @param string $pattern
* @return bool
*/
public function isSearcheable($pattern) {

This comment has been minimized.

Sign in to view

Copy link
@PVince81

PVince81 Mar 14, 2018

Contributor

Good thing that these are reusable functions, however it feels that these do not really belong in the UserManager. Are there other good candidates to move these to ?

This comment has been minimized.

Sign in to view

Copy link
@VicDeo

VicDeo Mar 15, 2018

Author Member

@PVince81 no other ideas except a separate SearchLimiter class

This comment has been minimized.

Sign in to view

Copy link
@PVince81

PVince81 Mar 16, 2018

Contributor

There is another option: add the check on a higher level, the caller of these functions. The problem is that there are likely a lot of those to find and cover, and that would also require a utility function somewhere.

How about moving these two methods to \OCP\Util ? :-S

This comment has been minimized.

Sign in to view

Copy link
@VicDeo

VicDeo Mar 16, 2018

Author Member

static flavor. it's kind of wrong...
$trimmed = trim($pattern);
return $trimmed === '' || strlen($trimmed) >= $this->getSearchMinLength();
}

public function getSearchMinLength() {
return $this->config->getSystemValue('user.search_min_length', 4);
}

/**
Expand Down

0 comments on commit 2f08494

Please sign in to comment.