Skip to content

Commit

Permalink
Cleanup AllConfig
Browse files Browse the repository at this point in the history 
- Port to QueryBuilder
- More typing when possible
- Import classes with 'use'

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
  • Loading branch information
@CarlSchwan
CarlSchwan committed Apr 22, 2022
1 parent 3f55108 commit 8bbdd9e
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 78 deletions.
133 changes: 64 additions & 69 deletions lib/private/AllConfig.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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;
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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);
}


Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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]);
Expand All @@ -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]);
}
Expand All @@ -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]);
Expand Down Expand Up @@ -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])) {
Expand Down Expand Up @@ -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'];
Expand All @@ -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()) {
Expand All @@ -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()) {
Expand Down
9 changes: 0 additions & 9 deletions tests/lib/AllConfigTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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');
Expand Down

0 comments on commit 8bbdd9e

Please sign in to comment.