From 8bbdd9ea0ce9678720459037e30239dc176b9ed8 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Fri, 22 Apr 2022 12:20:21 +0200 Subject: [PATCH] Cleanup AllConfig - Port to QueryBuilder - More typing when possible - Import classes with 'use' Signed-off-by: Carl Schwan --- lib/private/AllConfig.php | 133 +++++++++++++++++------------------- tests/lib/AllConfigTest.php | 9 --- 2 files changed, 64 insertions(+), 78 deletions(-) diff --git a/lib/private/AllConfig.php b/lib/private/AllConfig.php index e5a6e6a5acd59..f282baee14614 100644 --- a/lib/private/AllConfig.php +++ b/lib/private/AllConfig.php @@ -33,18 +33,17 @@ namespace OC; use OC\Cache\CappedMemoryCache; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IConfig; use OCP\IDBConnection; use OCP\PreConditionNotMetException; /** * Class to combine all the configuration options ownCloud offers */ -class AllConfig implements \OCP\IConfig { - /** @var SystemConfig */ - private $systemConfig; - - /** @var IDBConnection */ - private $connection; +class AllConfig implements IConfig { + private SystemConfig $systemConfig; + private ?IDBConnection $connection = null; /** * 3 dimensional array with the following structure: @@ -66,11 +65,8 @@ class AllConfig implements \OCP\IConfig { * * @var CappedMemoryCache $userCache */ - private $userCache; + private CappedMemoryCache $userCache; - /** - * @param SystemConfig $systemConfig - */ public function __construct(SystemConfig $systemConfig) { $this->userCache = new CappedMemoryCache(); $this->systemConfig = $systemConfig; @@ -91,7 +87,7 @@ public function __construct(SystemConfig $systemConfig) { */ private function fixDIInit() { if ($this->connection === null) { - $this->connection = \OC::$server->getDatabaseConnection(); + $this->connection = \OC::$server->get(IDBConnection::class); } } @@ -195,7 +191,7 @@ public function deleteSystemValue($key) { * @return string[] the keys stored for the app */ public function getAppKeys($appName) { - return \OC::$server->query(\OC\AppConfig::class)->getKeys($appName); + return \OC::$server->get(AppConfig::class)->getKeys($appName); } /** @@ -206,7 +202,7 @@ public function getAppKeys($appName) { * @param string|float|int $value the value that should be stored */ public function setAppValue($appName, $key, $value) { - \OC::$server->query(\OC\AppConfig::class)->setValue($appName, $key, $value); + \OC::$server->get(AppConfig::class)->setValue($appName, $key, $value); } /** @@ -218,7 +214,7 @@ public function setAppValue($appName, $key, $value) { * @return string the saved value */ public function getAppValue($appName, $key, $default = '') { - return \OC::$server->query(\OC\AppConfig::class)->getValue($appName, $key, $default); + return \OC::$server->get(AppConfig::class)->getValue($appName, $key, $default); } /** @@ -228,7 +224,7 @@ public function getAppValue($appName, $key, $default = '') { * @param string $key the key of the value, under which it was saved */ public function deleteAppValue($appName, $key) { - \OC::$server->query(\OC\AppConfig::class)->deleteKey($appName, $key); + \OC::$server->get(AppConfig::class)->deleteKey($appName, $key); } /** @@ -237,7 +233,7 @@ public function deleteAppValue($appName, $key) { * @param string $appName the appName the configs are stored under */ public function deleteAppValues($appName) { - \OC::$server->query(\OC\AppConfig::class)->deleteApp($appName); + \OC::$server->get(AppConfig::class)->deleteApp($appName); } @@ -278,7 +274,7 @@ public function setUserValue($userId, $appName, $key, $value, $preCondition = nu ->where($qb->expr()->eq('userid', $qb->createNamedParameter($userId))) ->andWhere($qb->expr()->eq('appid', $qb->createNamedParameter($appName))) ->andWhere($qb->expr()->eq('configkey', $qb->createNamedParameter($key))); - $qb->execute(); + $qb->executeStatement(); $this->userCache[$userId][$appName][$key] = (string)$value; return; @@ -354,9 +350,12 @@ public function deleteUserValue($userId, $appName, $key) { // TODO - FIXME $this->fixDIInit(); - $sql = 'DELETE FROM `*PREFIX*preferences` '. - 'WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?'; - $this->connection->executeUpdate($sql, [$userId, $appName, $key]); + $qb = $this->connection->getQueryBuilder(); + $qb->delete('preferences') + ->where($qb->expr()->eq('userid', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR))) + ->where($qb->expr()->eq('appid', $qb->createNamedParameter($appName, IQueryBuilder::PARAM_STR))) + ->where($qb->expr()->eq('configkey', $qb->createNamedParameter($key, IQueryBuilder::PARAM_STR))) + ->executeStatement(); if (isset($this->userCache[$userId][$appName])) { unset($this->userCache[$userId][$appName][$key]); @@ -371,10 +370,10 @@ public function deleteUserValue($userId, $appName, $key) { public function deleteAllUserValues($userId) { // TODO - FIXME $this->fixDIInit(); - - $sql = 'DELETE FROM `*PREFIX*preferences` '. - 'WHERE `userid` = ?'; - $this->connection->executeUpdate($sql, [$userId]); + $qb = $this->connection->getQueryBuilder(); + $qb->delete('preferences') + ->where($qb->expr()->eq('userid', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR))) + ->executeStatement(); unset($this->userCache[$userId]); } @@ -388,9 +387,10 @@ public function deleteAppFromAllUsers($appName) { // TODO - FIXME $this->fixDIInit(); - $sql = 'DELETE FROM `*PREFIX*preferences` '. - 'WHERE `appid` = ?'; - $this->connection->executeUpdate($sql, [$appName]); + $qb = $this->connection->getQueryBuilder(); + $qb->delete('preferences') + ->where($qb->expr()->eq('appid', $qb->createNamedParameter($appName, IQueryBuilder::PARAM_STR))) + ->executeStatement(); foreach ($this->userCache as &$userCache) { unset($userCache[$appName]); @@ -420,8 +420,12 @@ public function getAllUserValues(?string $userId): array { $this->fixDIInit(); $data = []; - $query = 'SELECT `appid`, `configkey`, `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ?'; - $result = $this->connection->executeQuery($query, [$userId]); + + $qb = $this->connection->getQueryBuilder(); + $result = $qb->select('appid', 'configkey', 'configvalue') + ->from('preferences') + ->where($qb->expr()->eq('userid', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR))) + ->executeQuery(); while ($row = $result->fetch()) { $appId = $row['appid']; if (!isset($data[$appId])) { @@ -450,22 +454,20 @@ public function getUserValueForUsers($appName, $key, $userIds) { } $chunkedUsers = array_chunk($userIds, 50, true); - $placeholders50 = implode(',', array_fill(0, 50, '?')); + + $qb = $this->connection->getQueryBuilder(); + $qb->select('userid', 'configvalue') + ->from('preferences') + ->where($qb->expr()->eq('appid', $qb->createParameter('appName'))) + ->andWhere($qb->expr()->eq('configkey', $qb->createParameter('configKey'))) + ->andWhere($qb->expr()->in('userid', $qb->createParameter('userIds'))); $userValues = []; foreach ($chunkedUsers as $chunk) { - $queryParams = $chunk; - // create [$app, $key, $chunkedUsers] - array_unshift($queryParams, $key); - array_unshift($queryParams, $appName); - - $placeholders = (count($chunk) === 50) ? $placeholders50 : implode(',', array_fill(0, count($chunk), '?')); - - $query = 'SELECT `userid`, `configvalue` ' . - 'FROM `*PREFIX*preferences` ' . - 'WHERE `appid` = ? AND `configkey` = ? ' . - 'AND `userid` IN (' . $placeholders . ')'; - $result = $this->connection->executeQuery($query, $queryParams); + $qb->setParameter('appName', $appName, IQueryBuilder::PARAM_STR); + $qb->setParameter('configKey', $key, IQueryBuilder::PARAM_STR); + $qb->setParameter('userIds', $chunk, IQueryBuilder::PARAM_STR_ARRAY); + $result = $qb->executeQuery(); while ($row = $result->fetch()) { $userValues[$row['userid']] = $row['configvalue']; @@ -487,19 +489,16 @@ public function getUsersForUserValue($appName, $key, $value) { // TODO - FIXME $this->fixDIInit(); - $sql = 'SELECT `userid` FROM `*PREFIX*preferences` ' . - 'WHERE `appid` = ? AND `configkey` = ? '; - - if ($this->getSystemValue('dbtype', 'sqlite') === 'oci') { - //oracle hack: need to explicitly cast CLOB to CHAR for comparison - $sql .= 'AND to_char(`configvalue`) = ?'; - } else { - $sql .= 'AND `configvalue` = ?'; - } - - $sql .= ' ORDER BY `userid`'; - - $result = $this->connection->executeQuery($sql, [$appName, $key, $value]); + $qb = $this->connection->getQueryBuilder(); + $result = $qb->select('userid') + ->from('preferences') + ->where($qb->expr()->eq('appid', $qb->createNamedParameter($appName, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('configkey', $qb->createNamedParameter($key, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq( + $qb->expr()->castColumn('configvalue', IQueryBuilder::PARAM_STR), + $qb->createNamedParameter($value, IQueryBuilder::PARAM_STR)) + )->orderBy('userid') + ->executeQuery(); $userIDs = []; while ($row = $result->fetch()) { @@ -525,20 +524,16 @@ public function getUsersForUserValueCaseInsensitive($appName, $key, $value) { // Email address is always stored lowercase in the database return $this->getUsersForUserValue($appName, $key, strtolower($value)); } - - $sql = 'SELECT `userid` FROM `*PREFIX*preferences` ' . - 'WHERE `appid` = ? AND `configkey` = ? '; - - if ($this->getSystemValue('dbtype', 'sqlite') === 'oci') { - //oracle hack: need to explicitly cast CLOB to CHAR for comparison - $sql .= 'AND LOWER(to_char(`configvalue`)) = ?'; - } else { - $sql .= 'AND LOWER(`configvalue`) = ?'; - } - - $sql .= ' ORDER BY `userid`'; - - $result = $this->connection->executeQuery($sql, [$appName, $key, strtolower($value)]); + $qb = $this->connection->getQueryBuilder(); + $result = $qb->select('userid') + ->from('preferences') + ->where($qb->expr()->eq('appid', $qb->createNamedParameter($appName, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('configkey', $qb->createNamedParameter($key, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq( + $qb->func()->lower($qb->expr()->castColumn('configvalue', IQueryBuilder::PARAM_STR)), + $qb->createNamedParameter(strtolower($value), IQueryBuilder::PARAM_STR)) + )->orderBy('userid') + ->executeQuery(); $userIDs = []; while ($row = $result->fetch()) { diff --git a/tests/lib/AllConfigTest.php b/tests/lib/AllConfigTest.php index b0b0b7eff8b50..8570b94456ff4 100644 --- a/tests/lib/AllConfigTest.php +++ b/tests/lib/AllConfigTest.php @@ -409,11 +409,6 @@ public function testGetUsersForUserValue() { $systemConfig = $this->getMockBuilder('\OC\SystemConfig') ->disableOriginalConstructor() ->getMock(); - $systemConfig->expects($this->once()) - ->method('getValue') - ->with($this->equalTo('dbtype'), - $this->equalTo('sqlite')) - ->willReturn(\OC::$server->getConfig()->getSystemValue('dbtype', 'sqlite')); $config = $this->getConfig($systemConfig); // preparation - add something to the database @@ -443,10 +438,6 @@ public function testGetUsersForUserValue() { public function testGetUsersForUserValueCaseInsensitive() { // mock the check for the database to run the correct SQL statements for each database type $systemConfig = $this->createMock(SystemConfig::class); - $systemConfig->expects($this->once()) - ->method('getValue') - ->with($this->equalTo('dbtype'), $this->equalTo('sqlite')) - ->willReturn(\OC::$server->getConfig()->getSystemValue('dbtype', 'sqlite')); $config = $this->getConfig($systemConfig); $config->setUserValue('user1', 'myApp', 'myKey', 'test123');