introduce preCondition for setUserValue to provide atomic check-and-update

This commit is contained in:
Morris Jobke 2014-12-03 21:31:29 +01:00
parent f0b10324ca
commit af91ee97c9
6 changed files with 152 additions and 14 deletions

View File

@ -9,6 +9,7 @@
namespace OC; namespace OC;
use OCP\IDBConnection; use OCP\IDBConnection;
use OCP\PreConditionNotMetException;
/** /**
* Class to combine all the configuration options ownCloud offers * Class to combine all the configuration options ownCloud offers
@ -140,8 +141,10 @@ class AllConfig implements \OCP\IConfig {
* @param string $appName the appName that we want to store the value under * @param string $appName the appName that we want to store the value under
* @param string $key the key under which the value is being stored * @param string $key the key under which the value is being stored
* @param string $value the value that you want to store * @param string $value the value that you want to store
* @param string $preCondition only update if the config value was previously the value passed as $preCondition
* @throws \OCP\PreConditionNotMetException if a precondition is specified and is not met
*/ */
public function setUserValue($userId, $appName, $key, $value) { public function setUserValue($userId, $appName, $key, $value, $preCondition = null) {
// Check if the key does exist // Check if the key does exist
$sql = 'SELECT `configvalue` FROM `*PREFIX*preferences` '. $sql = 'SELECT `configvalue` FROM `*PREFIX*preferences` '.
'WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?'; 'WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?';
@ -154,15 +157,24 @@ class AllConfig implements \OCP\IConfig {
return; return;
} }
if (!$exists) { $data = array($value, $userId, $appName, $key);
if (!$exists && $preCondition === null) {
$sql = 'INSERT INTO `*PREFIX*preferences` (`configvalue`, `userid`, `appid`, `configkey`)'. $sql = 'INSERT INTO `*PREFIX*preferences` (`configvalue`, `userid`, `appid`, `configkey`)'.
'VALUES (?, ?, ?, ?)'; 'VALUES (?, ?, ?, ?)';
} else { } elseif ($exists) {
$sql = 'UPDATE `*PREFIX*preferences` SET `configvalue` = ? '. $sql = 'UPDATE `*PREFIX*preferences` SET `configvalue` = ? '.
'WHERE `userid` = ? AND `appid` = ? AND `configkey` = ? '; 'WHERE `userid` = ? AND `appid` = ? AND `configkey` = ? ';
if($preCondition !== null) {
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` = ?';
}
$data[] = $preCondition;
}
} }
$data = array($value, $userId, $appName, $key);
$affectedRows = $this->connection->executeUpdate($sql, $data); $affectedRows = $this->connection->executeUpdate($sql, $data);
// only add to the cache if we already loaded data for the user // only add to the cache if we already loaded data for the user
@ -172,6 +184,10 @@ class AllConfig implements \OCP\IConfig {
} }
$this->userCache[$userId][$appName][$key] = $value; $this->userCache[$userId][$appName][$key] = $value;
} }
if ($preCondition !== null && $affectedRows === 0) {
throw new PreConditionNotMetException;
}
} }
/** /**

View File

@ -70,10 +70,12 @@ class OC_Preferences{
* will be added automagically. * will be added automagically.
*/ */
public static function setValue( $user, $app, $key, $value, $preCondition = null ) { public static function setValue( $user, $app, $key, $value, $preCondition = null ) {
return \OC::$server->getConfig()->setUserValue($user, $app, $key, $value); try {
\OC::$server->getConfig()->setUserValue($user, $app, $key, $value, $preCondition);
// TODO maybe catch exceptions and then return false
return true; return true;
} catch(\OCP\PreConditionNotMetException $e) {
return false;
}
} }
/** /**

View File

@ -37,6 +37,7 @@
namespace OC; namespace OC;
use OCP\IDBConnection; use OCP\IDBConnection;
use OCP\PreConditionNotMetException;
/** /**
@ -111,10 +112,12 @@ class Preferences {
* will be added automagically. * will be added automagically.
*/ */
public function setValue($user, $app, $key, $value, $preCondition = null) { public function setValue($user, $app, $key, $value, $preCondition = null) {
return $this->config->setUserValue($user, $app, $key, $value); try {
$this->config->setUserValue($user, $app, $key, $value, $preCondition);
// TODO maybe catch exceptions and then return false
return true; return true;
} catch(PreConditionNotMetException $e) {
return false;
}
} }
/** /**

View File

@ -109,8 +109,10 @@ interface IConfig {
* @param string $appName the appName that we want to store the value under * @param string $appName the appName that we want to store the value under
* @param string $key the key under which the value is being stored * @param string $key the key under which the value is being stored
* @param string $value the value that you want to store * @param string $value the value that you want to store
* @param string $preCondition only update if the config value was previously the value passed as $preCondition
* @throws \OCP\PreConditionNotMetException if a precondition is specified and is not met
*/ */
public function setUserValue($userId, $appName, $key, $value); public function setUserValue($userId, $appName, $key, $value, $preCondition = null);
/** /**
* Shortcut for getting a user defined value * Shortcut for getting a user defined value

View File

@ -0,0 +1,30 @@
<?php
/**
* ownCloud
*
* @author Morris Jobke
* @copyright 2014 Morris Jobke <hey@morrisjobke.de>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
* License as published by the Free Software Foundation; either
* version 3 of the License, or any later version.
*
* This library 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 library. If not, see <http://www.gnu.org/licenses/>.
*
*/
// use OCP namespace for all classes that are considered public.
// This means that they should be used by apps instead of the internal ownCloud classes
namespace OCP;
/**
* Exception if the precondition of the config update method isn't met
*/
class PreConditionNotMetException extends \Exception {}

View File

@ -80,6 +80,91 @@ class TestAllConfig extends \Test\TestCase {
$config->deleteUserValue('userSet', 'appSet', 'keySet'); $config->deleteUserValue('userSet', 'appSet', 'keySet');
} }
public function testSetUserValueWithPreCondition() {
// mock the check for the database to run the correct SQL statements for each database type
$systemConfig = $this->getMock('\OC\SystemConfig');
$systemConfig->expects($this->once())
->method('getValue')
->with($this->equalTo('dbtype'),
$this->equalTo('sqlite'))
->will($this->returnValue(\OC::$server->getConfig()->getSystemValue('dbtype', 'sqlite')));
$config = $this->getConfig($systemConfig);
$selectAllSQL = 'SELECT `userid`, `appid`, `configkey`, `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ?';
$config->setUserValue('userPreCond', 'appPreCond', 'keyPreCond', 'valuePreCond');
$result = $this->connection->executeQuery($selectAllSQL, array('userPreCond'))->fetchAll();
$this->assertEquals(1, count($result));
$this->assertEquals(array(
'userid' => 'userPreCond',
'appid' => 'appPreCond',
'configkey' => 'keyPreCond',
'configvalue' => 'valuePreCond'
), $result[0]);
// test if the method overwrites existing database entries with valid precond
$config->setUserValue('userPreCond', 'appPreCond', 'keyPreCond', 'valuePreCond2', 'valuePreCond');
$result = $this->connection->executeQuery($selectAllSQL, array('userPreCond'))->fetchAll();
$this->assertEquals(1, count($result));
$this->assertEquals(array(
'userid' => 'userPreCond',
'appid' => 'appPreCond',
'configkey' => 'keyPreCond',
'configvalue' => 'valuePreCond2'
), $result[0]);
// cleanup
$config->deleteUserValue('userPreCond', 'appPreCond', 'keyPreCond');
}
/**
* @expectedException \OCP\PreConditionNotMetException
*/
public function testSetUserValueWithPreConditionFailure() {
// mock the check for the database to run the correct SQL statements for each database type
$systemConfig = $this->getMock('\OC\SystemConfig');
$systemConfig->expects($this->once())
->method('getValue')
->with($this->equalTo('dbtype'),
$this->equalTo('sqlite'))
->will($this->returnValue(\OC::$server->getConfig()->getSystemValue('dbtype', 'sqlite')));
$config = $this->getConfig($systemConfig);
$selectAllSQL = 'SELECT `userid`, `appid`, `configkey`, `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ?';
$config->setUserValue('userPreCond1', 'appPreCond', 'keyPreCond', 'valuePreCond');
$result = $this->connection->executeQuery($selectAllSQL, array('userPreCond1'))->fetchAll();
$this->assertEquals(1, count($result));
$this->assertEquals(array(
'userid' => 'userPreCond1',
'appid' => 'appPreCond',
'configkey' => 'keyPreCond',
'configvalue' => 'valuePreCond'
), $result[0]);
// test if the method overwrites existing database entries with valid precond
$config->setUserValue('userPreCond1', 'appPreCond', 'keyPreCond', 'valuePreCond2', 'valuePreCond3');
$result = $this->connection->executeQuery($selectAllSQL, array('userPreCond1'))->fetchAll();
$this->assertEquals(1, count($result));
$this->assertEquals(array(
'userid' => 'userPreCond1',
'appid' => 'appPreCond',
'configkey' => 'keyPreCond',
'configvalue' => 'valuePreCond'
), $result[0]);
// cleanup
$config->deleteUserValue('userPreCond1', 'appPreCond', 'keyPreCond');
}
public function testSetUserValueUnchanged() { public function testSetUserValueUnchanged() {
$resultMock = $this->getMockBuilder('\Doctrine\DBAL\Driver\Statement') $resultMock = $this->getMockBuilder('\Doctrine\DBAL\Driver\Statement')
->disableOriginalConstructor()->getMock(); ->disableOriginalConstructor()->getMock();