Improve search results when only phonebook-matches can we autocompleted

Signed-off-by: Joas Schilling <coding@schilljs.com>
This commit is contained in:
Joas Schilling 2021-03-17 09:02:37 +01:00
parent f82edda9c1
commit 9a189bc710
No known key found for this signature in database
GPG Key ID: 7076EA9751AACDDA
9 changed files with 189 additions and 11 deletions

View File

@ -534,6 +534,7 @@ return array(
'OCP\\User\\Backend\\IGetRealUIDBackend' => $baseDir . '/lib/public/User/Backend/IGetRealUIDBackend.php', 'OCP\\User\\Backend\\IGetRealUIDBackend' => $baseDir . '/lib/public/User/Backend/IGetRealUIDBackend.php',
'OCP\\User\\Backend\\IPasswordConfirmationBackend' => $baseDir . '/lib/public/User/Backend/IPasswordConfirmationBackend.php', 'OCP\\User\\Backend\\IPasswordConfirmationBackend' => $baseDir . '/lib/public/User/Backend/IPasswordConfirmationBackend.php',
'OCP\\User\\Backend\\IProvideAvatarBackend' => $baseDir . '/lib/public/User/Backend/IProvideAvatarBackend.php', 'OCP\\User\\Backend\\IProvideAvatarBackend' => $baseDir . '/lib/public/User/Backend/IProvideAvatarBackend.php',
'OCP\\User\\Backend\\ISearchKnownUsersBackend' => $baseDir . '/lib/public/User/Backend/ISearchKnownUsersBackend.php',
'OCP\\User\\Backend\\ISetDisplayNameBackend' => $baseDir . '/lib/public/User/Backend/ISetDisplayNameBackend.php', 'OCP\\User\\Backend\\ISetDisplayNameBackend' => $baseDir . '/lib/public/User/Backend/ISetDisplayNameBackend.php',
'OCP\\User\\Backend\\ISetPasswordBackend' => $baseDir . '/lib/public/User/Backend/ISetPasswordBackend.php', 'OCP\\User\\Backend\\ISetPasswordBackend' => $baseDir . '/lib/public/User/Backend/ISetPasswordBackend.php',
'OCP\\User\\Events\\BeforePasswordUpdatedEvent' => $baseDir . '/lib/public/User/Events/BeforePasswordUpdatedEvent.php', 'OCP\\User\\Events\\BeforePasswordUpdatedEvent' => $baseDir . '/lib/public/User/Events/BeforePasswordUpdatedEvent.php',

View File

@ -563,6 +563,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OCP\\User\\Backend\\IGetRealUIDBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IGetRealUIDBackend.php', 'OCP\\User\\Backend\\IGetRealUIDBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IGetRealUIDBackend.php',
'OCP\\User\\Backend\\IPasswordConfirmationBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IPasswordConfirmationBackend.php', 'OCP\\User\\Backend\\IPasswordConfirmationBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IPasswordConfirmationBackend.php',
'OCP\\User\\Backend\\IProvideAvatarBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IProvideAvatarBackend.php', 'OCP\\User\\Backend\\IProvideAvatarBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IProvideAvatarBackend.php',
'OCP\\User\\Backend\\ISearchKnownUsersBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/ISearchKnownUsersBackend.php',
'OCP\\User\\Backend\\ISetDisplayNameBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/ISetDisplayNameBackend.php', 'OCP\\User\\Backend\\ISetDisplayNameBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/ISetDisplayNameBackend.php',
'OCP\\User\\Backend\\ISetPasswordBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/ISetPasswordBackend.php', 'OCP\\User\\Backend\\ISetPasswordBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/ISetPasswordBackend.php',
'OCP\\User\\Events\\BeforePasswordUpdatedEvent' => __DIR__ . '/../../..' . '/lib/public/User/Events/BeforePasswordUpdatedEvent.php', 'OCP\\User\\Events\\BeforePasswordUpdatedEvent' => __DIR__ . '/../../..' . '/lib/public/User/Events/BeforePasswordUpdatedEvent.php',

View File

@ -116,7 +116,11 @@ class UserPlugin implements ISearchPlugin {
} }
} else { } else {
// Search in all users // Search in all users
$usersTmp = $this->userManager->searchDisplayName($search, $limit, $offset); if ($this->shareeEnumerationPhone) {
$usersTmp = $this->userManager->searchKnownUsersByDisplayName($currentUserId, $search, $limit, $offset);
} else {
$usersTmp = $this->userManager->searchDisplayName($search, $limit, $offset);
}
foreach ($usersTmp as $user) { foreach ($usersTmp as $user) {
if ($user->isEnabled()) { // Don't keep deactivated users if ($user->isEnabled()) { // Don't keep deactivated users
$users[$user->getUID()] = $user; $users[$user->getUID()] = $user;

View File

@ -70,6 +70,7 @@ use OCP\User\Backend\ICreateUserBackend;
use OCP\User\Backend\IGetDisplayNameBackend; use OCP\User\Backend\IGetDisplayNameBackend;
use OCP\User\Backend\IGetHomeBackend; use OCP\User\Backend\IGetHomeBackend;
use OCP\User\Backend\IGetRealUIDBackend; use OCP\User\Backend\IGetRealUIDBackend;
use OCP\User\Backend\ISearchKnownUsersBackend;
use OCP\User\Backend\ISetDisplayNameBackend; use OCP\User\Backend\ISetDisplayNameBackend;
use OCP\User\Backend\ISetPasswordBackend; use OCP\User\Backend\ISetPasswordBackend;
@ -84,6 +85,7 @@ class Database extends ABackend implements
ICheckPasswordBackend, ICheckPasswordBackend,
IGetHomeBackend, IGetHomeBackend,
ICountUsersBackend, ICountUsersBackend,
ISearchKnownUsersBackend,
IGetRealUIDBackend { IGetRealUIDBackend {
/** @var CappedMemoryCache */ /** @var CappedMemoryCache */
private $cache; private $cache;
@ -277,7 +279,47 @@ class Database extends ABackend implements
->orWhere($query->expr()->iLike('displayname', $query->createPositionalParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%'))) ->orWhere($query->expr()->iLike('displayname', $query->createPositionalParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%')))
->orWhere($query->expr()->iLike('configvalue', $query->createPositionalParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%'))) ->orWhere($query->expr()->iLike('configvalue', $query->createPositionalParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%')))
->orderBy($query->func()->lower('displayname'), 'ASC') ->orderBy($query->func()->lower('displayname'), 'ASC')
->orderBy('uid_lower', 'ASC') ->addOrderBy('uid_lower', 'ASC')
->setMaxResults($limit)
->setFirstResult($offset);
$result = $query->execute();
$displayNames = [];
while ($row = $result->fetch()) {
$displayNames[(string)$row['uid']] = (string)$row['displayname'];
}
return $displayNames;
}
/**
* @param string $searcher
* @param string $pattern
* @param int|null $limit
* @param int|null $offset
* @return array
* @since 21.0.1
*/
public function searchKnownUsersByDisplayName(string $searcher, string $pattern, ?int $limit = null, ?int $offset = null): array {
$limit = $this->fixLimit($limit);
$this->fixDI();
$query = $this->dbConn->getQueryBuilder();
$query->select('u.uid', 'u.displayname')
->from($this->table, 'u')
->leftJoin('u', 'known_users', 'k', $query->expr()->andX(
$query->expr()->eq('k.known_user', 'u.uid'),
$query->expr()->eq('k.known_to', $query->createNamedParameter($searcher))
))
->where($query->expr()->eq('k.known_to', $query->createNamedParameter($searcher)))
->andWhere($query->expr()->orX(
$query->expr()->iLike('u.uid', $query->createNamedParameter('%' . $this->dbConn->escapeLikeParameter($pattern) . '%')),
$query->expr()->iLike('u.displayname', $query->createNamedParameter('%' . $this->dbConn->escapeLikeParameter($pattern) . '%'))
))
->orderBy('u.displayname', 'ASC')
->addOrderBy('u.uid_lower', 'ASC')
->setMaxResults($limit) ->setMaxResults($limit)
->setFirstResult($offset); ->setFirstResult($offset);

View File

@ -47,6 +47,7 @@ use OCP\IUserBackend;
use OCP\IUserManager; use OCP\IUserManager;
use OCP\Support\Subscription\IRegistry; use OCP\Support\Subscription\IRegistry;
use OCP\User\Backend\IGetRealUIDBackend; use OCP\User\Backend\IGetRealUIDBackend;
use OCP\User\Backend\ISearchKnownUsersBackend;
use OCP\User\Events\BeforeUserCreatedEvent; use OCP\User\Events\BeforeUserCreatedEvent;
use OCP\User\Events\UserCreatedEvent; use OCP\User\Events\UserCreatedEvent;
use OCP\UserInterface; use OCP\UserInterface;
@ -329,6 +330,41 @@ class Manager extends PublicEmitter implements IUserManager {
return $users; return $users;
} }
/**
* Search known users (from phonebook sync) by displayName
*
* @param string $searcher
* @param string $pattern
* @param int|null $limit
* @param int|null $offset
* @return IUser[]
*/
public function searchKnownUsersByDisplayName(string $searcher, string $pattern, ?int $limit = null, ?int $offset = null): array {
$users = [];
foreach ($this->backends as $backend) {
if ($backend instanceof ISearchKnownUsersBackend) {
$backendUsers = $backend->searchKnownUsersByDisplayName($searcher, $pattern, $limit, $offset);
} else {
// Better than nothing, but filtering after pagination can remove lots of results.
$backendUsers = $backend->getDisplayNames($pattern, $limit, $offset);
}
if (is_array($backendUsers)) {
foreach ($backendUsers as $uid => $displayName) {
$users[] = $this->getUserObject($uid, $backend);
}
}
}
usort($users, function ($a, $b) {
/**
* @var IUser $a
* @var IUser $b
*/
return strcasecmp($a->getDisplayName(), $b->getDisplayName());
});
return $users;
}
/** /**
* @param string $uid * @param string $uid
* @param string $password * @param string $password

View File

@ -126,6 +126,18 @@ interface IUserManager {
*/ */
public function searchDisplayName($pattern, $limit = null, $offset = null); public function searchDisplayName($pattern, $limit = null, $offset = null);
/**
* Search known users (from phonebook sync) by displayName
*
* @param string $searcher
* @param string $pattern
* @param int|null $limit
* @param int|null $offset
* @return IUser[]
* @since 21.0.1
*/
public function searchKnownUsersByDisplayName(string $searcher, string $pattern, ?int $limit = null, ?int $offset = null): array;
/** /**
* @param string $uid * @param string $uid
* @param string $password * @param string $password

View File

@ -33,7 +33,7 @@ interface IGetRealUIDBackend {
/** /**
* Some backends accept different UIDs than what is the internal UID to be used. * Some backends accept different UIDs than what is the internal UID to be used.
* For example the database backend accepts differnt cased UIDs in all the functions * For example the database backend accepts different cased UIDs in all the functions
* but the internal UID that is to be used should be correctly cased. * but the internal UID that is to be used should be correctly cased.
* *
* This little function makes sure that the used UID will be correct hen using the user object * This little function makes sure that the used UID will be correct hen using the user object

View File

@ -0,0 +1,43 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2021 Joas Schilling <coding@schilljs.com>
*
* @author Joas Schilling <coding@schilljs.com>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* 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
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OCP\User\Backend;
/**
* @since 21.0.1
*/
interface ISearchKnownUsersBackend {
/**
* @param string $searcher
* @param string $pattern
* @param int|null $limit
* @param int|null $offset
* @return array
* @since 21.0.1
*/
public function searchKnownUsersByDisplayName(string $searcher, string $pattern, ?int $limit = null, ?int $offset = null): array;
}

View File

@ -104,17 +104,19 @@ class UserPluginTest extends TestCase {
); );
} }
public function mockConfig($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup) { public function mockConfig($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup, $shareeEnumerationPhone = false) {
$this->config->expects($this->any()) $this->config->expects($this->any())
->method('getAppValue') ->method('getAppValue')
->willReturnCallback( ->willReturnCallback(
function ($appName, $key, $default) use ($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup) { function ($appName, $key, $default) use ($shareWithGroupOnly, $shareeEnumeration, $shareeEnumerationLimitToGroup, $shareeEnumerationPhone) {
if ($appName === 'core' && $key === 'shareapi_only_share_with_group_members') { if ($appName === 'core' && $key === 'shareapi_only_share_with_group_members') {
return $shareWithGroupOnly ? 'yes' : 'no'; return $shareWithGroupOnly ? 'yes' : 'no';
} elseif ($appName === 'core' && $key === 'shareapi_allow_share_dialog_user_enumeration') { } elseif ($appName === 'core' && $key === 'shareapi_allow_share_dialog_user_enumeration') {
return $shareeEnumeration ? 'yes' : 'no'; return $shareeEnumeration ? 'yes' : 'no';
} elseif ($appName === 'core' && $key === 'shareapi_restrict_user_enumeration_to_group') { } elseif ($appName === 'core' && $key === 'shareapi_restrict_user_enumeration_to_group') {
return $shareeEnumerationLimitToGroup ? 'yes' : 'no'; return $shareeEnumerationLimitToGroup ? 'yes' : 'no';
} elseif ($appName === 'core' && $key === 'shareapi_restrict_user_enumeration_to_phone') {
return $shareeEnumerationPhone ? 'yes' : 'no';
} }
return $default; return $default;
} }
@ -266,6 +268,28 @@ class UserPluginTest extends TestCase {
false, false,
false, false,
], ],
[
'test',
false,
true,
[],
[
$this->getUserMock('test0', 'Test'),
$this->getUserMock('test1', 'Test One'),
$this->getUserMock('test2', 'Test Two'),
],
[
['label' => 'Test', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test0'], 'icon' => 'icon-user', 'subline' => null, 'status' => [], 'shareWithDisplayNameUnique' => 'test0'],
],
[
['label' => 'Test One', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test1'], 'icon' => 'icon-user', 'subline' => null, 'status' => [], 'shareWithDisplayNameUnique' => 'test1'],
['label' => 'Test Two', 'value' => ['shareType' => IShare::TYPE_USER, 'shareWith' => 'test2'], 'icon' => 'icon-user', 'subline' => null, 'status' => [], 'shareWithDisplayNameUnique' => 'test2'],
],
false,
false,
[],
true,
],
[ [
'test', 'test',
false, false,
@ -443,9 +467,10 @@ class UserPluginTest extends TestCase {
array $expected, array $expected,
$reachedEnd, $reachedEnd,
$singleUser, $singleUser,
array $users = [] array $users = [],
$shareeEnumerationPhone = false
) { ) {
$this->mockConfig($shareWithGroupOnly, $shareeEnumeration, false); $this->mockConfig($shareWithGroupOnly, $shareeEnumeration, false, $shareeEnumerationPhone);
$this->instantiatePlugin(); $this->instantiatePlugin();
$this->session->expects($this->any()) $this->session->expects($this->any())
@ -453,10 +478,24 @@ class UserPluginTest extends TestCase {
->willReturn($this->user); ->willReturn($this->user);
if (!$shareWithGroupOnly) { if (!$shareWithGroupOnly) {
$this->userManager->expects($this->once()) if ($shareeEnumerationPhone) {
->method('searchDisplayName') $this->userManager->expects($this->once())
->with($searchTerm, $this->limit, $this->offset) ->method('searchKnownUsersByDisplayName')
->willReturn($userResponse); ->with($this->user->getUID(), $searchTerm, $this->limit, $this->offset)
->willReturn($userResponse);
$this->knownUserService->method('isKnownToUser')
->willReturnMap([
[$this->user->getUID(), 'test0', true],
[$this->user->getUID(), 'test1', true],
[$this->user->getUID(), 'test2', true],
]);
} else {
$this->userManager->expects($this->once())
->method('searchDisplayName')
->with($searchTerm, $this->limit, $this->offset)
->willReturn($userResponse);
}
} else { } else {
$this->groupManager->method('getUserGroupIds') $this->groupManager->method('getUserGroupIds')
->with($this->user) ->with($this->user)