From 88cd61521459a18599407f83347f1d6a0e7700cc Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Mon, 24 Aug 2015 13:21:09 +0100 Subject: [PATCH] Introduce IDBConnection::setValues() setValues() attempts to insert a new row, or failing that, update an existing row. The ability to set preconditions is also available. --- lib/private/allconfig.php | 55 +++++------------ lib/private/appframework/db/db.php | 15 +++++ lib/private/db/connection.php | 47 +++++++++++++++ lib/public/idbconnection.php | 14 +++++ tests/lib/allconfig.php | 22 +------ tests/lib/db/connection.php | 94 ++++++++++++++++++++++++++++-- 6 files changed, 179 insertions(+), 68 deletions(-) diff --git a/lib/private/allconfig.php b/lib/private/allconfig.php index af7ffa4168..3c85bfacbb 100644 --- a/lib/private/allconfig.php +++ b/lib/private/allconfig.php @@ -205,57 +205,28 @@ class AllConfig implements \OCP\IConfig { // TODO - FIXME $this->fixDIInit(); - // Check if the key does exist - $sql = 'SELECT `configvalue` FROM `*PREFIX*preferences` '. - 'WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?'; - $result = $this->connection->executeQuery($sql, array($userId, $appName, $key)); - $oldValue = $result->fetchColumn(); - $result->closeCursor(); - $exists = $oldValue !== false; - - if($oldValue === strval($value)) { - // no changes - return; + $preconditionArray = []; + if (isset($preCondition)) { + $preconditionArray = [ + 'configvalue' => $preCondition, + ]; } - $affectedRows = 0; - if (!$exists && $preCondition === null) { - $this->connection->insertIfNotExist('*PREFIX*preferences', [ - 'configvalue' => $value, - 'userid' => $userId, - 'appid' => $appName, - 'configkey' => $key, - ], ['configkey', 'userid', 'appid']); - $affectedRows = 1; - } elseif ($exists) { - $data = array($value, $userId, $appName, $key); - - $sql = 'UPDATE `*PREFIX*preferences` SET `configvalue` = ? '. - '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; - } - $affectedRows = $this->connection->executeUpdate($sql, $data); - } + $this->connection->setValues('preferences', [ + 'userid' => $userId, + 'appid' => $appName, + 'configkey' => $key, + ], [ + 'configvalue' => $value, + ], $preconditionArray); // only add to the cache if we already loaded data for the user - if ($affectedRows > 0 && isset($this->userCache[$userId])) { + if (isset($this->userCache[$userId])) { if (!isset($this->userCache[$userId][$appName])) { $this->userCache[$userId][$appName] = array(); } $this->userCache[$userId][$appName][$key] = $value; } - - if ($preCondition !== null && $affectedRows === 0) { - throw new PreConditionNotMetException; - } } /** diff --git a/lib/private/appframework/db/db.php b/lib/private/appframework/db/db.php index 812649daa7..5fdc5d1066 100644 --- a/lib/private/appframework/db/db.php +++ b/lib/private/appframework/db/db.php @@ -146,6 +146,21 @@ class Db implements IDb { return $this->connection->insertIfNotExist($table, $input, $compare); } + /** + * Insert or update a row value + * + * @param string $table + * @param array $keys (column name => value) + * @param array $values (column name => value) + * @param array $updatePreconditionValues ensure values match preconditions (column name => value) + * @return int number of new rows + * @throws \Doctrine\DBAL\DBALException + * @throws PreconditionNotMetException + */ + public function setValues($table, array $keys, array $values, array $updatePreconditionValues = []) { + return $this->connection->setValues($table, $keys, $values, $updatePreconditionValues); + } + /** * Start a transaction */ diff --git a/lib/private/db/connection.php b/lib/private/db/connection.php index 28bf3b6e05..b1c1c12cb4 100644 --- a/lib/private/db/connection.php +++ b/lib/private/db/connection.php @@ -32,6 +32,7 @@ use Doctrine\Common\EventManager; use OC\DB\QueryBuilder\ExpressionBuilder; use OC\DB\QueryBuilder\QueryBuilder; use OCP\IDBConnection; +use OCP\PreconditionNotMetException; class Connection extends \Doctrine\DBAL\Connection implements IDBConnection { /** @@ -241,6 +242,52 @@ class Connection extends \Doctrine\DBAL\Connection implements IDBConnection { return $this->adapter->insertIfNotExist($table, $input, $compare); } + /** + * Insert or update a row value + * + * @param string $table + * @param array $keys (column name => value) + * @param array $values (column name => value) + * @param array $updatePreconditionValues ensure values match preconditions (column name => value) + * @return int number of new rows + * @throws \Doctrine\DBAL\DBALException + * @throws PreconditionNotMetException + */ + public function setValues($table, array $keys, array $values, array $updatePreconditionValues = []) { + try { + $insertQb = $this->getQueryBuilder(); + $insertQb->insert($table) + ->values( + array_map(function($value) use ($insertQb) { + return $insertQb->createNamedParameter($value); + }, array_merge($keys, $values)) + ); + return $insertQb->execute(); + } catch (\Doctrine\DBAL\Exception\UniqueConstraintViolationException $e) { + // value already exists, try update + $updateQb = $this->getQueryBuilder(); + $updateQb->update($table); + foreach ($values as $name => $value) { + $updateQb->set($name, $updateQb->createNamedParameter($value)); + } + $where = $updateQb->expr()->andx(); + $whereValues = array_merge($keys, $updatePreconditionValues); + foreach ($whereValues as $name => $value) { + $where->add($updateQb->expr()->eq( + $name, $updateQb->createNamedParameter($value) + )); + } + $updateQb->where($where); + $affected = $updateQb->execute(); + + if ($affected === 0) { + throw new PreconditionNotMetException(); + } + + return 0; + } + } + /** * returns the error code and message as a string for logging * works with DoctrineException diff --git a/lib/public/idbconnection.php b/lib/public/idbconnection.php index 49856fb78a..c5767e65a8 100644 --- a/lib/public/idbconnection.php +++ b/lib/public/idbconnection.php @@ -108,6 +108,20 @@ interface IDBConnection { */ public function insertIfNotExist($table, $input, array $compare = null); + /** + * Insert or update a row value + * + * @param string $table + * @param array $keys (column name => value) + * @param array $values (column name => value) + * @param array $updatePreconditionValues ensure values match preconditions (column name => value) + * @return int number of new rows + * @throws \Doctrine\DBAL\DBALException + * @throws PreconditionNotMetException + * @since 9.0.0 + */ + public function setValues($table, array $keys, array $values, array $updatePreconditionValues = []); + /** * Start a transaction * @since 6.0.0 diff --git a/tests/lib/allconfig.php b/tests/lib/allconfig.php index 0caf8163cf..869bf9b8d6 100644 --- a/tests/lib/allconfig.php +++ b/tests/lib/allconfig.php @@ -90,16 +90,7 @@ class TestAllConfig extends \Test\TestCase { } public function testSetUserValueWithPreCondition() { - // mock the check for the database to run the correct SQL statements for each database type - $systemConfig = $this->getMockBuilder('\OC\SystemConfig') - ->disableOriginalConstructor() - ->getMock(); - $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); + $config = $this->getConfig(); $selectAllSQL = 'SELECT `userid`, `appid`, `configkey`, `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ?'; @@ -136,16 +127,7 @@ class TestAllConfig extends \Test\TestCase { * @expectedException \OCP\PreConditionNotMetException */ public function testSetUserValueWithPreConditionFailure() { - // mock the check for the database to run the correct SQL statements for each database type - $systemConfig = $this->getMockBuilder('\OC\SystemConfig') - ->disableOriginalConstructor() - ->getMock(); - $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); + $config = $this->getConfig(); $selectAllSQL = 'SELECT `userid`, `appid`, `configkey`, `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ?'; diff --git a/tests/lib/db/connection.php b/tests/lib/db/connection.php index 348a5e31e0..51b4145b7a 100644 --- a/tests/lib/db/connection.php +++ b/tests/lib/db/connection.php @@ -25,20 +25,17 @@ class Connection extends \Test\TestCase { */ private $connection; - public static function setUpBeforeClass() - { + public static function setUpBeforeClass() { self::dropTestTable(); parent::setUpBeforeClass(); } - public static function tearDownAfterClass() - { + public static function tearDownAfterClass() { self::dropTestTable(); parent::tearDownAfterClass(); } - protected static function dropTestTable() - { + protected static function dropTestTable() { if (\OC::$server->getConfig()->getSystemValue('dbtype', 'sqlite') !== 'oci') { \OC::$server->getDatabaseConnection()->dropTable('table'); } @@ -92,4 +89,89 @@ class Connection extends \Test\TestCase { $this->connection->dropTable('table'); $this->assertTableNotExist('table'); } + + private function getTextValueByIntergerField($integerField) { + $builder = $this->connection->getQueryBuilder(); + $query = $builder->select('textfield') + ->from('table') + ->where($builder->expr()->eq('integerfield', $builder->createNamedParameter($integerField, \PDO::PARAM_INT))); + + $result = $query->execute(); + return $result->fetchColumn(); + } + + public function testSetValues() { + $this->makeTestTable(); + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'foo' + ]); + + $this->assertEquals('foo', $this->getTextValueByIntergerField(1)); + + $this->connection->dropTable('table'); + } + + public function testSetValuesOverWrite() { + $this->makeTestTable(); + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'foo' + ]); + + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'bar' + ]); + + $this->assertEquals('bar', $this->getTextValueByIntergerField(1)); + + $this->connection->dropTable('table'); + } + + public function testSetValuesOverWritePrecondition() { + $this->makeTestTable(); + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'foo', + 'booleanfield' => true + ]); + + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'bar' + ], [ + 'booleanfield' => true + ]); + + $this->assertEquals('bar', $this->getTextValueByIntergerField(1)); + + $this->connection->dropTable('table'); + } + + /** + * @expectedException \OCP\PreConditionNotMetException + */ + public function testSetValuesOverWritePreconditionFailed() { + $this->makeTestTable(); + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'foo', + 'booleanfield' => true + ]); + + $this->connection->setValues('table', [ + 'integerfield' => 1 + ], [ + 'textfield' => 'bar' + ], [ + 'booleanfield' => false + ]); + } }