diff --git a/config/config.sample.php b/config/config.sample.php index 00f307e46d4b..36edd171c1e5 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -259,9 +259,10 @@ /** * Allow medial search on account properties like display name, user id, email, * and other search terms. Allows finding 'Alice' when searching for 'lic'. - * May slow down user search. + * May slow down user search. Disable this if you encounter slow username search + * in the sharing dialog. */ -'accounts.enable_medial_search' => false, +'accounts.enable_medial_search' => true, /** * Mail Parameters diff --git a/core/Migrations/Version20170221114437.php b/core/Migrations/Version20170221114437.php index 036993e9ed18..1ceebec287de 100644 --- a/core/Migrations/Version20170221114437.php +++ b/core/Migrations/Version20170221114437.php @@ -6,6 +6,7 @@ use OC\User\AccountTermMapper; use OC\User\Database; use OC\User\SyncService; +use OCP\IConfig; use OCP\Migration\ISimpleMigration; use OCP\Migration\IOutput; @@ -19,7 +20,7 @@ public function run(IOutput $out) { $config = \OC::$server->getConfig(); $logger = \OC::$server->getLogger(); $connection = \OC::$server->getDatabaseConnection(); - $accountMapper = new AccountMapper($connection, new AccountTermMapper($connection)); + $accountMapper = new AccountMapper($config, $connection, new AccountTermMapper($connection)); $syncService = new SyncService($accountMapper, $backend, $config, $logger); // insert/update known users diff --git a/lib/private/Server.php b/lib/private/Server.php index 83b2d73c875a..aa30b6862800 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -222,7 +222,7 @@ public function __construct($webRoot, \OC\Config $config) { }); }); $this->registerService('AccountMapper', function(Server $c) { - return new AccountMapper($c->getDatabaseConnection(), new AccountTermMapper($c->getDatabaseConnection())); + return new AccountMapper($c->getConfig(), $c->getDatabaseConnection(), new AccountTermMapper($c->getDatabaseConnection())); }); $this->registerService('UserManager', function (Server $c) { $config = $c->getConfig(); diff --git a/lib/private/User/AccountMapper.php b/lib/private/User/AccountMapper.php index 0e8a6edba616..0817f9542dc0 100644 --- a/lib/private/User/AccountMapper.php +++ b/lib/private/User/AccountMapper.php @@ -32,11 +32,15 @@ class AccountMapper extends Mapper { + /* @var IConfig */ + protected $config; + /* @var AccountTermMapper */ protected $termMapper; - public function __construct(IDBConnection $db, AccountTermMapper $termMapper) { + public function __construct(IConfig $config, IDBConnection $db, AccountTermMapper $termMapper) { parent::__construct($db, 'accounts', Account::class); + $this->config = $config; $this->termMapper = $termMapper; } @@ -135,7 +139,6 @@ public function search($fieldName, $pattern, $limit, $offset) { $qb = $this->db->getQueryBuilder(); $qb->select('*') ->from($this->getTableName()) - // TODO check performance on large installs because like with starting % cannot use indexes ->where($qb->expr()->iLike($fieldName, $qb->createNamedParameter('%' . $this->db->escapeLikeParameter($pattern) . '%'))) ->orderBy($fieldName); @@ -149,17 +152,26 @@ public function search($fieldName, $pattern, $limit, $offset) { * @return Account[] */ public function find($pattern, $limit = null, $offset = null) { - $lowerPattern = strtolower($pattern); + + $allowMedialSearches = $this->config->getSystemValue('accounts.enable_medial_search', true); + if ($allowMedialSearches) { + $parameter = '%' . $this->db->escapeLikeParameter($pattern) . '%'; + $loweredParameter = '%' . $this->db->escapeLikeParameter(strtolower($pattern)) . '%'; + } else { + $parameter = $this->db->escapeLikeParameter($pattern) . '%'; + $loweredParameter = $this->db->escapeLikeParameter(strtolower($pattern)) . '%'; + } + $qb = $this->db->getQueryBuilder(); $qb->selectAlias('DISTINCT a.id', 'id') ->addSelect(['user_id', 'lower_user_id', 'display_name', 'email', 'last_login', 'backend', 'state', 'quota', 'home']) ->from($this->getTableName(), 'a') ->leftJoin('a', 'account_terms', 't', $qb->expr()->eq('a.id', 't.account_id')) ->orderBy('display_name') - ->where($qb->expr()->like('lower_user_id', $qb->createNamedParameter($this->db->escapeLikeParameter($lowerPattern) . '%'))) - ->orWhere($qb->expr()->iLike('display_name', $qb->createNamedParameter($this->db->escapeLikeParameter($pattern) . '%'))) - ->orWhere($qb->expr()->iLike('email', $qb->createNamedParameter($this->db->escapeLikeParameter($pattern) . '%'))) - ->orWhere($qb->expr()->like('t.term', $qb->createNamedParameter($this->db->escapeLikeParameter($lowerPattern) . '%'))); + ->where($qb->expr()->like('lower_user_id', $qb->createNamedParameter($loweredParameter))) + ->orWhere($qb->expr()->iLike('display_name', $qb->createNamedParameter($parameter))) + ->orWhere($qb->expr()->iLike('email', $qb->createNamedParameter($parameter))) + ->orWhere($qb->expr()->like('t.term', $qb->createNamedParameter($loweredParameter))); return $this->findEntities($qb->getSQL(), $qb->getParameters(), $limit, $offset); } @@ -209,7 +221,6 @@ public function callForAllUsers($callback, $search, $onlySeen) { if ($search) { $qb->where($qb->expr()->iLike('user_id', - // TODO check performance on large installs because like with starting % cannot use indexes $qb->createNamedParameter('%' . $this->db->escapeLikeParameter($search) . '%'))); } if ($onlySeen) { diff --git a/tests/lib/Traits/UserTrait.php b/tests/lib/Traits/UserTrait.php index 171de3a32c7c..a9612c37d2a4 100644 --- a/tests/lib/Traits/UserTrait.php +++ b/tests/lib/Traits/UserTrait.php @@ -10,6 +10,7 @@ use OC\User\AccountTermMapper; use OC\User\User; +use OCP\IConfig; use Test\Util\User\Dummy; use Test\Util\User\MemoryAccountMapper; @@ -39,7 +40,8 @@ protected function createUser($name, $password = null) { protected function setUpUserTrait() { $db = \OC::$server->getDatabaseConnection(); - $accountMapper = new MemoryAccountMapper($db, new AccountTermMapper($db)); + $config = $this->createMock(IConfig::class); + $accountMapper = new MemoryAccountMapper($config, $db, new AccountTermMapper($db)); $accountMapper->testCaseName = get_class($this); $this->previousUserManagerInternals = \OC::$server->getUserManager() ->reset($accountMapper, [Dummy::class => new Dummy()]); diff --git a/tests/lib/User/AccountMapperTest.php b/tests/lib/User/AccountMapperTest.php index 7b0cd66c0e77..85b6607c8ac1 100644 --- a/tests/lib/User/AccountMapperTest.php +++ b/tests/lib/User/AccountMapperTest.php @@ -25,6 +25,7 @@ use OC\User\Account; use OC\User\AccountMapper; use OC\User\AccountTermMapper; +use OCP\IConfig; use OCP\IDBConnection; use Test\TestCase; @@ -37,13 +38,13 @@ */ class AccountMapperTest extends TestCase { - /** - * @var IDBConnection - */ + /** @var IConfig | \PHPUnit_Framework_MockObject_MockObject */ + protected $config; + + /** @var IDBConnection */ protected $connection; - /** - * @var AccountMapper - */ + + /** @var AccountMapper */ protected $mapper; public static function setUpBeforeClass() { @@ -76,8 +77,16 @@ public static function setUpBeforeClass() { public function setUp() { parent::setUp(); + + $this->config = $this->createMock(IConfig::class); + $this->connection = \OC::$server->getDatabaseConnection(); - $this->mapper = new AccountMapper($this->connection, new AccountTermMapper($this->connection)); + + $this->mapper = new AccountMapper( + $this->config, + $this->connection, + new AccountTermMapper($this->connection) + ); } public static function tearDownAfterClass () { @@ -91,7 +100,6 @@ public static function tearDownAfterClass () { public function testFindAll() { $result = $this->mapper->find("testfind"); $this->assertEquals(4, count($result)); - } /** @@ -101,7 +109,6 @@ public function testFindByUserId() { $result = $this->mapper->find("testfind1"); $this->assertEquals(1, count($result)); $this->assertEquals("TestFind1", array_shift($result)->getUserId()); - } /** @@ -111,7 +118,6 @@ public function testFindByDisplayName() { $result = $this->mapper->find('test find 2'); $this->assertEquals(1, count($result)); $this->assertEquals("TestFind2", array_shift($result)->getUserId()); - } /** @@ -121,7 +127,6 @@ public function testFindByEmail() { $result= $this->mapper->find('test3@find.tld'); $this->assertEquals(1, count($result)); $this->assertEquals("TestFind3", array_shift($result)->getUserId()); - } /** @@ -131,7 +136,6 @@ public function testFindBySearchTerm() { $result = $this->mapper->find('term 4 b'); $this->assertEquals(1, count($result)); $this->assertEquals("TestFind4", array_shift($result)->getUserId()); - } /** @@ -143,6 +147,5 @@ public function testFindLimitAndOffset() { //results are ordered by display name $this->assertEquals("TestFind3", array_shift($result)->getUserId()); $this->assertEquals("TestFind4", array_shift($result)->getUserId()); - } } \ No newline at end of file