Cleanup AllConfig

- Port to QueryBuilder
- More typing when possible
- Import classes with 'use'

Signed-off-by: Carl Schwan <carl@carlschwan.eu>

diff --git a/lib/private/AllConfig.php b/lib/private/AllConfig.php
index e5a6e6a5acd5969bbc6ae6c773e3c5bba78bee0a..f282baee14614ea6658d0addab1bc751895d245c 100644 (file)
--- 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 @@ class AllConfig implements \OCP\IConfig {
         */
        private function fixDIInit() {
                if ($this->connection === null) {
-                       $this->connection = \OC::$server->getDatabaseConnection();
+                       $this->connection = \OC::$server->get(IDBConnection::class);
                }
        }
 
@@ -195,7 +191,7 @@ class AllConfig implements \OCP\IConfig {
         * @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 @@ class AllConfig implements \OCP\IConfig {
         * @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 @@ class AllConfig implements \OCP\IConfig {
         * @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 @@ class AllConfig implements \OCP\IConfig {
         * @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 @@ class AllConfig implements \OCP\IConfig {
         * @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 @@ class AllConfig implements \OCP\IConfig {
                                        ->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 @@ class AllConfig implements \OCP\IConfig {
                // 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 @@ class AllConfig implements \OCP\IConfig {
        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 @@ class AllConfig implements \OCP\IConfig {
                // 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 @@ class AllConfig implements \OCP\IConfig {
                $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 @@ class AllConfig implements \OCP\IConfig {
                }
 
                $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 @@ class AllConfig implements \OCP\IConfig {
                // 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 @@ class AllConfig implements \OCP\IConfig {
                        // 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 b0b0b7eff8b5051d048fab484caf05e838f64a27..8570b94456ff4f7111bf93d62f15e5263f73f8cf 100644 (file)
--- a/tests/lib/AllConfigTest.php
+++ b/tests/lib/AllConfigTest.php
@@ -409,11 +409,6 @@ class AllConfigTest extends \Test\TestCase {
                $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 @@ class AllConfigTest extends \Test\TestCase {
        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');