Merge pull request #9076 from nextcloud/feature/7573/user_db_backend_index

Make select queries on user DB backend indexable
2018-04-06 14:56:19 +02:00 · 2018-04-06 14:56:19 +02:00 · e970e9f710
parent ca644c768f 31454c2072
commit e970e9f710
5 changed files with 148 additions and 25 deletions
--- a/core/Migrations/Version14000Date20180404140050.php
+++ b/core/Migrations/Version14000Date20180404140050.php
@ -0,0 +1,80 @@
+<?php
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2018 Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @author Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @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 OC\Core\Migrations;
+
+use OCP\DB\ISchemaWrapper;
+use OCP\IDBConnection;
+use OCP\Migration\SimpleMigrationStep;
+use OCP\Migration\IOutput;
+
+/**
+ * Auto-generated migration step: Please modify to your needs!
+ */
+class Version14000Date20180404140050 extends SimpleMigrationStep {
+
+	/** @var IDBConnection */
+	private $connection;
+
+	public function __construct(IDBConnection $connection) {
+		$this->connection = $connection;
+	}
+
+	/**
+	 * @param IOutput $output
+	 * @param \Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
+	 * @param array $options
+	 * @return null|ISchemaWrapper
+	 */
+	public function changeSchema(IOutput $output, \Closure $schemaClosure, array $options) {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		$table = $schema->getTable('users');
+
+		$table->addColumn('uid_lower', 'string', [
+			'notnull' => false,
+			'length' => 64,
+			'default' => '',
+		]);
+		$table->addIndex(['uid_lower'], 'user_uid_lower');
+
+		return $schema;
+	}
+
+	/**
+	 * @param IOutput $output
+	 * @param \Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
+	 * @param array $options
+	 *
+	 * @suppress SqlInjectionChecker
+	 */
+	public function postSchemaChange(IOutput $output, \Closure $schemaClosure, array $options) {
+		$qb = $this->connection->getQueryBuilder();
+
+		$qb->update('users')
+			->set('uid_lower', $qb->createFunction('LOWER(`uid`)'));
+		$qb->execute();
+	}
+}
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@ -547,6 +547,7 @@ return array(
    'OC\\Core\\Migrations\\Version13000Date20170919121250' => $baseDir . '/core/Migrations/Version13000Date20170919121250.php',
    'OC\\Core\\Migrations\\Version13000Date20170926101637' => $baseDir . '/core/Migrations/Version13000Date20170926101637.php',
    'OC\\Core\\Migrations\\Version14000Date20180129121024' => $baseDir . '/core/Migrations/Version14000Date20180129121024.php',
+    'OC\\Core\\Migrations\\Version14000Date20180404140050' => $baseDir . '/core/Migrations/Version14000Date20180404140050.php',
    'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php',
    'OC\\DB\\AdapterMySQL' => $baseDir . '/lib/private/DB/AdapterMySQL.php',
    'OC\\DB\\AdapterOCI8' => $baseDir . '/lib/private/DB/AdapterOCI8.php',
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@ -577,6 +577,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
        'OC\\Core\\Migrations\\Version13000Date20170919121250' => __DIR__ . '/../../..' . '/core/Migrations/Version13000Date20170919121250.php',
        'OC\\Core\\Migrations\\Version13000Date20170926101637' => __DIR__ . '/../../..' . '/core/Migrations/Version13000Date20170926101637.php',
        'OC\\Core\\Migrations\\Version14000Date20180129121024' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180129121024.php',
+        'OC\\Core\\Migrations\\Version14000Date20180404140050' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180404140050.php',
        'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php',
        'OC\\DB\\AdapterMySQL' => __DIR__ . '/../../..' . '/lib/private/DB/AdapterMySQL.php',
        'OC\\DB\\AdapterOCI8' => __DIR__ . '/../../..' . '/lib/private/DB/AdapterOCI8.php',
--- a/lib/private/User/Database.php
+++ b/lib/private/User/Database.php
@ -58,6 +58,7 @@ declare(strict_types=1);
 namespace OC\User;

 use OC\Cache\CappedMemoryCache;
+use OCP\IDBConnection;
 use OCP\User\Backend\ABackend;
 use OCP\User\Backend\ICheckPasswordBackend;
 use OCP\User\Backend\ICountUsersBackend;
@ -87,6 +88,9 @@ class Database extends ABackend
 	/** @var EventDispatcher */
 	private $eventDispatcher;

+	/** @var IDBConnection */
+	private $dbConn;
+
 	/**
 	 * \OC\User\Database constructor.
 	 *
@ -97,6 +101,15 @@ class Database extends ABackend
 		$this->eventDispatcher = $eventDispatcher ? $eventDispatcher : \OC::$server->getEventDispatcher();
 	}

+	/**
+	 * FIXME: This function should not be required!
+	 */
+	private function fixDI() {
+		if ($this->dbConn === null) {
+			$this->dbConn = \OC::$server->getDatabaseConnection();
+		}
+	}
+
 	/**
 	 * Create a new user
 	 *
@ -108,15 +121,21 @@ class Database extends ABackend
 	 * itself, not in its subclasses.
 	 */
 	public function createUser(string $uid, string $password): bool {
+		$this->fixDI();
+
 		if (!$this->userExists($uid)) {
 			$event = new GenericEvent($password);
 			$this->eventDispatcher->dispatch('OCP\PasswordPolicy::validate', $event);
-			$query = \OC_DB::prepare('INSERT INTO `*PREFIX*users` ( `uid`, `password` ) VALUES( ?, ? )');
-			try {
-				$result = $query->execute([$uid, \OC::$server->getHasher()->hash($password)]);
-			} catch (\Exception $e) {
-				$result = false;
-			}
+
+			$qb = $this->dbConn->getQueryBuilder();
+			$qb->insert('users')
+				->values([
+					'uid' => $qb->createNamedParameter($uid),
+					'password' => $qb->createNamedParameter(\OC::$server->getHasher()->hash($password)),
+					'uid_lower' => $qb->createNamedParameter(mb_strtolower($uid)),
+				]);
+
+			$result = $qb->execute();

 			// Clear cache
 			unset($this->cache[$uid]);
@ -136,6 +155,8 @@ class Database extends ABackend
 	 * Deletes a user
 	 */
 	public function deleteUser($uid) {
+		$this->fixDI();
+
 		// Delete user-group-relation
 		$query = \OC_DB::prepare('DELETE FROM `*PREFIX*users` WHERE `uid` = ?');
 		$result = $query->execute([$uid]);
@ -157,6 +178,8 @@ class Database extends ABackend
 	 * Change the password of a user
 	 */
 	public function setPassword(string $uid, string $password): bool {
+		$this->fixDI();
+
 		if ($this->userExists($uid)) {
 			$event = new GenericEvent($password);
 			$this->eventDispatcher->dispatch('OCP\PasswordPolicy::validate', $event);
@ -179,6 +202,8 @@ class Database extends ABackend
 	 * Change the display name of a user
 	 */
 	public function setDisplayName(string $uid, string $displayName): bool {
+		$this->fixDI();
+
 		if ($this->userExists($uid)) {
 			$query = \OC_DB::prepare('UPDATE `*PREFIX*users` SET `displayname` = ? WHERE LOWER(`uid`) = LOWER(?)');
 			$query->execute([$displayName, $uid]);
@ -210,9 +235,9 @@ class Database extends ABackend
 	 * @return array an array of all displayNames (value) and the corresponding uids (key)
 	 */
 	public function getDisplayNames($search = '', $limit = null, $offset = null) {
-		$connection = \OC::$server->getDatabaseConnection();
+		$this->fixDI();

-		$query = $connection->getQueryBuilder();
+		$query = $this->dbConn->getQueryBuilder();

 		$query->select('uid', 'displayname')
 			->from('users', 'u')
@ -222,9 +247,9 @@ class Database extends ABackend
 				$query->expr()->eq('configkey', $query->expr()->literal('email')))
 			)
 			// sqlite doesn't like re-using a single named parameter here
-			->where($query->expr()->iLike('uid', $query->createPositionalParameter('%' . $connection->escapeLikeParameter($search) . '%')))
-			->orWhere($query->expr()->iLike('displayname', $query->createPositionalParameter('%' . $connection->escapeLikeParameter($search) . '%')))
-			->orWhere($query->expr()->iLike('configvalue', $query->createPositionalParameter('%' . $connection->escapeLikeParameter($search) . '%')))
+			->where($query->expr()->iLike('uid', $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) . '%')))
 			->orderBy($query->func()->lower('displayname'), 'ASC')
 			->orderBy($query->func()->lower('uid'), 'ASC')
 			->setMaxResults($limit)
@ -250,10 +275,20 @@ class Database extends ABackend
 	 * returns the user id or false
 	 */
 	public function checkPassword(string $uid, string $password) {
-		$query = \OC_DB::prepare('SELECT `uid`, `password` FROM `*PREFIX*users` WHERE LOWER(`uid`) = LOWER(?)');
-		$result = $query->execute([$uid]);
+		$this->fixDI();
+
+		$qb = $this->dbConn->getQueryBuilder();
+		$qb->select('uid', 'password')
+			->from('users')
+			->where(
+				$qb->expr()->eq(
+					'uid_lower', $qb->createNamedParameter(mb_strtolower($uid))
+				)
+			);
+		$result = $qb->execute();
+		$row = $result->fetch();
+		$result->closeCursor();

-		$row = $result->fetchRow();
 		if ($row) {
 			$storedHash = $row['password'];
 			$newHash = '';
@ -276,6 +311,8 @@ class Database extends ABackend
 	 * @return boolean true if user was found, false otherwise
 	 */
 	private function loadUser($uid) {
+		$this->fixDI();
+
 		$uid = (string)$uid;
 		if (!isset($this->cache[$uid])) {
 			//guests $uid could be NULL or ''
@ -284,23 +321,25 @@ class Database extends ABackend
 				return true;
 			}

-			$query = \OC_DB::prepare('SELECT `uid`, `displayname` FROM `*PREFIX*users` WHERE LOWER(`uid`) = LOWER(?)');
-			$result = $query->execute([$uid]);
-
-			if ($result === false) {
-				Util::writeLog('core', \OC_DB::getErrorMessage(), Util::ERROR);
-				return false;
-			}
+			$qb = $this->dbConn->getQueryBuilder();
+			$qb->select('uid', 'displayname')
+				->from('users')
+				->where(
+					$qb->expr()->eq(
+						'uid_lower', $qb->createNamedParameter(mb_strtolower($uid))
+					)
+				);
+			$result = $qb->execute();
+			$row = $result->fetch();
+			$result->closeCursor();

 			$this->cache[$uid] = false;

 			// "uid" is primary key, so there can only be a single result
-			if ($row = $result->fetchRow()) {
+			if ($row !== false) {
 				$this->cache[$uid]['uid'] = $row['uid'];
 				$this->cache[$uid]['displayname'] = $row['displayname'];
-				$result->closeCursor();
 			} else {
-				$result->closeCursor();
 				return false;
 			}
 		}
@ -361,6 +400,8 @@ class Database extends ABackend
 	 * @return int|bool
 	 */
 	public function countUsers() {
+		$this->fixDI();
+
 		$query = \OC_DB::prepare('SELECT COUNT(*) FROM `*PREFIX*users`');
 		$result = $query->execute();
 		if ($result === false) {
--- a/version.php
+++ b/version.php
@ -29,7 +29,7 @@
 // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel
 // when updating major/minor version number.

-$OC_Version = array(14, 0, 0, 1);
+$OC_Version = array(14, 0, 0, 2);

 // The human readable string
 $OC_VersionString = '14.0.0 alpha';