diff --git a/lib/private/allconfig.php b/lib/private/allconfig.php index 00defd920d..b8bba7986e 100644 --- a/lib/private/allconfig.php +++ b/lib/private/allconfig.php @@ -189,11 +189,18 @@ class AllConfig implements \OCP\IConfig { return; } - $data = array($value, $userId, $appName, $key); + $affectedRows = 0; if (!$exists && $preCondition === null) { - $sql = 'INSERT INTO `*PREFIX*preferences` (`configvalue`, `userid`, `appid`, `configkey`)'. - 'VALUES (?, ?, ?, ?)'; + $this->connection->insertIfNotExist('*PREFIX*preferences', [ + 'configvalue' => $value, + 'userid' => $userId, + 'appid' => $appName, + 'configkey' => $key, + ], ['configvalue', 'userid', 'appid']); + $affectedRows = 1; } elseif ($exists) { + $data = array($value, $userId, $appName, $key); + $sql = 'UPDATE `*PREFIX*preferences` SET `configvalue` = ? '. 'WHERE `userid` = ? AND `appid` = ? AND `configkey` = ? '; @@ -206,8 +213,8 @@ class AllConfig implements \OCP\IConfig { } $data[] = $preCondition; } + $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 if ($affectedRows > 0 && isset($this->userCache[$userId])) { diff --git a/lib/private/appframework/db/db.php b/lib/private/appframework/db/db.php index 5387e36d62..0824e108f4 100644 --- a/lib/private/appframework/db/db.php +++ b/lib/private/appframework/db/db.php @@ -121,25 +121,17 @@ class Db implements IDb { } /** - * Insert a row if a matching row doesn't exists. - * @param string $table The table name (will replace *PREFIX*) to perform the replace on. - * @param array $input - * @throws \OC\HintException - * - * The input array if in the form: - * - * array ( 'id' => array ( 'value' => 6, - * 'key' => true - * ), - * 'name' => array ('value' => 'Stoyan'), - * 'family' => array ('value' => 'Stefanov'), - * 'birth_date' => array ('value' => '1975-06-20') - * ); - * @return bool + * Insert a row if the matching row does not exists. * + * @param string $table The table name (will replace *PREFIX* with the actual prefix) + * @param array $input data that should be inserted into the table (column name => value) + * @param array|null $compare List of values that should be checked for "if not exists" + * If this is null or an empty array, all keys of $input will be compared + * @return int number of inserted rows + * @throws \Doctrine\DBAL\DBALException */ - public function insertIfNotExist($table, $input) { - return $this->connection->insertIfNotExist($table, $input); + public function insertIfNotExist($table, $input, array $compare = null) { + return $this->connection->insertIfNotExist($table, $input, $compare); } /** diff --git a/lib/private/db.php b/lib/private/db.php index dc25092e27..c265492669 100644 --- a/lib/private/db.php +++ b/lib/private/db.php @@ -20,8 +20,6 @@ * */ -define('MDB2_SCHEMA_DUMP_STRUCTURE', '1'); - /** * This class manages the access to the database. It basically is a wrapper for * Doctrine with some adaptions. @@ -40,8 +38,7 @@ class OC_DB { * * @return \OC\DB\MDB2SchemaManager */ - private static function getMDB2SchemaManager() - { + private static function getMDB2SchemaManager() { return new \OC\DB\MDB2SchemaManager(\OC::$server->getDatabaseConnection()); } @@ -166,16 +163,6 @@ class OC_DB { return \OC::$server->getDatabaseConnection()->lastInsertId($table); } - /** - * Insert a row if a matching row doesn't exists. - * @param string $table The table to insert into in the form '*PREFIX*tableName' - * @param array $input An array of fieldname/value pairs - * @return boolean number of updated rows - */ - public static function insertIfNotExist($table, $input) { - return \OC::$server->getDatabaseConnection()->insertIfNotExist($table, $input); - } - /** * Start a transaction */ @@ -205,7 +192,7 @@ class OC_DB { * * TODO: write more documentation */ - public static function getDbStructure( $file, $mode = 0) { + public static function getDbStructure($file) { $schemaManager = self::getMDB2SchemaManager(); return $schemaManager->getDbStructure($file); } diff --git a/lib/private/db/adapter.php b/lib/private/db/adapter.php index 58b3514b92..de7b04c74d 100644 --- a/lib/private/db/adapter.php +++ b/lib/private/db/adapter.php @@ -40,44 +40,37 @@ class Adapter { } /** - * insert the @input values when they do not exist yet - * @param string $table name - * @param array $input key->value pair, key has to be sanitized properly - * @throws \OC\HintException - * @return int count of inserted rows + * Insert a row if the matching row does not exists. + * + * @param string $table The table name (will replace *PREFIX* with the actual prefix) + * @param array $input data that should be inserted into the table (column name => value) + * @param array|null $compare List of values that should be checked for "if not exists" + * If this is null or an empty array, all keys of $input will be compared + * @return int number of inserted rows + * @throws \Doctrine\DBAL\DBALException */ - public function insertIfNotExist($table, $input) { + public function insertIfNotExist($table, $input, array $compare = null) { + if (empty($compare)) { + $compare = array_keys($input); + } $query = 'INSERT INTO `' .$table . '` (`' . implode('`,`', array_keys($input)) . '`) SELECT ' . str_repeat('?,', count($input)-1).'? ' // Is there a prettier alternative? . 'FROM `' . $table . '` WHERE '; $inserts = array_values($input); - foreach($input as $key => $value) { + foreach($compare as $key) { $query .= '`' . $key . '`'; - if (is_null($value)) { + if (is_null($input[$key])) { $query .= ' IS NULL AND '; } else { - $inserts[] = $value; + $inserts[] = $input[$key]; $query .= ' = ? AND '; } } $query = substr($query, 0, strlen($query) - 5); $query .= ' HAVING COUNT(*) = 0'; - try { - return $this->conn->executeUpdate($query, $inserts); - } catch(\Doctrine\DBAL\DBALException $e) { - $entry = 'DB Error: "'.$e->getMessage() . '"
'; - $entry .= 'Offending command was: ' . $query.'
'; - \OC_Log::write('core', $entry, \OC_Log::FATAL); - $l = \OC::$server->getL10N('lib'); - throw new \OC\HintException( - $l->t('Database Error'), - $l->t('Please contact your system administrator.'), - 0, - $e - ); - } + return $this->conn->executeUpdate($query, $inserts); } } diff --git a/lib/private/db/adaptersqlite.php b/lib/private/db/adaptersqlite.php index df4a804feb..31f88940f0 100644 --- a/lib/private/db/adaptersqlite.php +++ b/lib/private/db/adaptersqlite.php @@ -18,38 +18,38 @@ class AdapterSqlite extends Adapter { return $statement; } - public function insertIfNotExist($table, $input) { + /** + * Insert a row if the matching row does not exists. + * + * @param string $table The table name (will replace *PREFIX* with the actual prefix) + * @param array $input data that should be inserted into the table (column name => value) + * @param array|null $compare List of values that should be checked for "if not exists" + * If this is null or an empty array, all keys of $input will be compared + * @return int number of inserted rows + * @throws \Doctrine\DBAL\DBALException + */ + public function insertIfNotExist($table, $input, array $compare = null) { + if (empty($compare)) { + $compare = array_keys($input); + } $fieldList = '`' . implode('`,`', array_keys($input)) . '`'; $query = "INSERT INTO `$table` ($fieldList) SELECT " . str_repeat('?,', count($input)-1).'? ' . " WHERE NOT EXISTS (SELECT 1 FROM `$table` WHERE "; $inserts = array_values($input); - foreach($input as $key => $value) { + foreach($compare as $key) { $query .= '`' . $key . '`'; - if (is_null($value)) { + if (is_null($input[$key])) { $query .= ' IS NULL AND '; } else { - $inserts[] = $value; + $inserts[] = $input[$key]; $query .= ' = ? AND '; } } $query = substr($query, 0, strlen($query) - 5); $query .= ')'; - try { - return $this->conn->executeUpdate($query, $inserts); - } catch(\Doctrine\DBAL\DBALException $e) { - $entry = 'DB Error: "'.$e->getMessage() . '"
'; - $entry .= 'Offending command was: ' . $query.'
'; - \OC_Log::write('core', $entry, \OC_Log::FATAL); - $l = \OC::$server->getL10N('lib'); - throw new \OC\HintException( - $l->t('Database Error'), - $l->t('Please contact your system administrator.'), - 0, - $e - ); - } + return $this->conn->executeUpdate($query, $inserts); } } diff --git a/lib/private/db/connection.php b/lib/private/db/connection.php index 6ba29fc2cc..023e265f24 100644 --- a/lib/private/db/connection.php +++ b/lib/private/db/connection.php @@ -152,20 +152,22 @@ class Connection extends \Doctrine\DBAL\Connection implements IDBConnection { } // internal use - public function realLastInsertId($seqName = null) - { + public function realLastInsertId($seqName = null) { return parent::lastInsertId($seqName); } /** - * Insert a row if a matching row doesn't exists. - * @param string $table. The table to insert into in the form '*PREFIX*tableName' - * @param array $input. An array of fieldname/value pairs - * @throws \OC\HintException - * @return bool The return value from execute() + * Insert a row if the matching row does not exists. + * + * @param string $table The table name (will replace *PREFIX* with the actual prefix) + * @param array $input data that should be inserted into the table (column name => value) + * @param array|null $compare List of values that should be checked for "if not exists" + * If this is null or an empty array, all keys of $input will be compared + * @return int number of inserted rows + * @throws \Doctrine\DBAL\DBALException */ - public function insertIfNotExist($table, $input) { - return $this->adapter->insertIfNotExist($table, $input); + public function insertIfNotExist($table, $input, array $compare = null) { + return $this->adapter->insertIfNotExist($table, $input, $compare); } /** diff --git a/lib/private/db/mdb2schemamanager.php b/lib/private/db/mdb2schemamanager.php index 358360d0b4..66c97f9e3b 100644 --- a/lib/private/db/mdb2schemamanager.php +++ b/lib/private/db/mdb2schemamanager.php @@ -35,7 +35,7 @@ class MDB2SchemaManager { * * TODO: write more documentation */ - public function getDbStructure($file, $mode = MDB2_SCHEMA_DUMP_STRUCTURE) { + public function getDbStructure($file) { return \OC_DB_MDB2SchemaWriter::saveSchemaToFile($file, $this->conn); } diff --git a/lib/private/files/cache/cache.php b/lib/private/files/cache/cache.php index 62c32ce659..64661ca115 100644 --- a/lib/private/files/cache/cache.php +++ b/lib/private/files/cache/cache.php @@ -214,6 +214,7 @@ class Cache { * @param array $data * * @return int file id + * @throws \RuntimeException */ public function put($file, array $data) { if (($id = $this->getId($file)) > -1) { @@ -251,11 +252,20 @@ class Cache { return trim($item, "`"); }, $queryParts); $values = array_combine($queryParts, $params); - if (\OC::$server->getDatabaseConnection()->insertIfNotExist('*PREFIX*filecache', $values)) { + if (\OC::$server->getDatabaseConnection()->insertIfNotExist('*PREFIX*filecache', $values, [ + 'storage', + 'path_hash', + ])) { return (int)\OC_DB::insertid('*PREFIX*filecache'); } - return $this->getId($file); + // The file was created in the mean time + if (($id = $this->getId($file)) > -1) { + $this->update($id, $data); + return $id; + } else { + throw new \RuntimeException('File entry exists when inserting and does not exist on select... go away'); + } } } diff --git a/lib/private/files/cache/storage.php b/lib/private/files/cache/storage.php index d7d57811a7..9f2739bbed 100644 --- a/lib/private/files/cache/storage.php +++ b/lib/private/files/cache/storage.php @@ -21,6 +21,7 @@ class Storage { /** * @param \OC\Files\Storage\Storage|string $storage + * @throws \RuntimeException */ public function __construct($storage) { if ($storage instanceof \OC\Files\Storage\Storage) { @@ -35,9 +36,17 @@ class Storage { if ($row = $result->fetchRow()) { $this->numericId = $row['numeric_id']; } else { - $sql = 'INSERT INTO `*PREFIX*storages` (`id`) VALUES(?)'; - \OC_DB::executeAudited($sql, array($this->storageId)); - $this->numericId = \OC_DB::insertid('*PREFIX*storages'); + $connection = \OC_DB::getConnection(); + if ($connection->insertIfNotExist('*PREFIX*storages', ['id' => $this->storageId])) { + $this->numericId = \OC_DB::insertid('*PREFIX*storages'); + } else { + $result = \OC_DB::executeAudited($sql, array($this->storageId)); + if ($row = $result->fetchRow()) { + $this->numericId = $row['numeric_id']; + } else { + throw new \RuntimeException('Storage exists when inserting and does not exist on select... go away'); + } + } } } diff --git a/lib/public/db.php b/lib/public/db.php index e8fc817106..595ecd66bd 100644 --- a/lib/public/db.php +++ b/lib/public/db.php @@ -48,24 +48,18 @@ class DB { } /** - * Insert a row if a matching row doesn't exists. - * @param string $table The optional table name (will replace *PREFIX*) and add sequence suffix - * @param array $input + * Insert a row if the matching row does not exists. * - * The input array if in the form: - * - * array ( 'id' => array ( 'value' => 6, - * 'key' => true - * ), - * 'name' => array ('value' => 'Stoyan'), - * 'family' => array ('value' => 'Stefanov'), - * 'birth_date' => array ('value' => '1975-06-20') - * ); - * @return bool + * @param string $table The table name (will replace *PREFIX* with the actual prefix) + * @param array $input data that should be inserted into the table (column name => value) + * @param array|null $compare List of values that should be checked for "if not exists" + * If this is null or an empty array, all keys of $input will be compared + * @return int number of inserted rows + * @throws \Doctrine\DBAL\DBALException * */ - public static function insertIfNotExist($table, $input) { - return(\OC_DB::insertIfNotExist($table, $input)); + public static function insertIfNotExist($table, $input, array $compare = null) { + return \OC::$server->getDatabaseConnection()->insertIfNotExist($table, $input, $compare); } /** diff --git a/lib/public/idbconnection.php b/lib/public/idbconnection.php index 0d3274d90e..1117c2da9b 100644 --- a/lib/public/idbconnection.php +++ b/lib/public/idbconnection.php @@ -77,24 +77,16 @@ interface IDBConnection { public function lastInsertId($table = null); /** - * Insert a row if a matching row doesn't exists. - * @param string $table The table name (will replace *PREFIX*) to perform the replace on. - * @param array $input - * @throws \OC\HintException - * - * The input array if in the form: - * - * array ( 'id' => array ( 'value' => 6, - * 'key' => true - * ), - * 'name' => array ('value' => 'Stoyan'), - * 'family' => array ('value' => 'Stefanov'), - * 'birth_date' => array ('value' => '1975-06-20') - * ); - * @return bool + * Insert a row if the matching row does not exists. * + * @param string $table The table name (will replace *PREFIX* with the actual prefix) + * @param array $input data that should be inserted into the table (column name => value) + * @param array|null $compare List of values that should be checked for "if not exists" + * If this is null or an empty array, all keys of $input will be compared + * @return int number of inserted rows + * @throws \Doctrine\DBAL\DBALException */ - public function insertIfNotExist($table, $input); + public function insertIfNotExist($table, $input, array $compare = null); /** * Start a transaction diff --git a/tests/data/db_structure.xml b/tests/data/db_structure.xml index 858c9ab100..371da94483 100644 --- a/tests/data/db_structure.xml +++ b/tests/data/db_structure.xml @@ -238,4 +238,59 @@ + + *dbprefix*uniconst + + + + id + integer + 0 + true + 1 + 4 + + + + + storage + integer + + true + 4 + + + + path_hash + text + + true + 32 + + + + etag + text + + false + 40 + + + + + true + + storage + ascending + + + path_hash + ascending + + + + + +
+ diff --git a/tests/lib/db.php b/tests/lib/db.php index aefbb3624e..1fb384ca9c 100644 --- a/tests/lib/db.php +++ b/tests/lib/db.php @@ -32,13 +32,18 @@ class Test_DB extends \Test\TestCase { */ private $table4; + /** + * @var string + */ + private $table5; + protected function setUp() { parent::setUp(); - $dbfile = OC::$SERVERROOT.'/tests/data/db_structure.xml'; + $dbFile = OC::$SERVERROOT.'/tests/data/db_structure.xml'; - $r = '_'.OC_Util::generateRandomBytes(4).'_'; - $content = file_get_contents( $dbfile ); + $r = $this->getUniqueID('_', 4).'_'; + $content = file_get_contents( $dbFile ); $content = str_replace( '*dbprefix*', '*dbprefix*'.$r, $content ); file_put_contents( self::$schema_file, $content ); OC_DB::createDbFromStructure(self::$schema_file); @@ -48,6 +53,7 @@ class Test_DB extends \Test\TestCase { $this->table2 = $this->test_prefix.'cntcts_cards'; $this->table3 = $this->test_prefix.'vcategory'; $this->table4 = $this->test_prefix.'decimal'; + $this->table5 = $this->test_prefix.'uniconst'; } protected function tearDown() { @@ -110,7 +116,7 @@ class Test_DB extends \Test\TestCase { } public function testinsertIfNotExist() { - $categoryentries = array( + $categoryEntries = array( array('user' => 'test', 'type' => 'contact', 'category' => 'Family', 'expectedResult' => 1), array('user' => 'test', 'type' => 'contact', 'category' => 'Friends', 'expectedResult' => 1), array('user' => 'test', 'type' => 'contact', 'category' => 'Coworkers', 'expectedResult' => 1), @@ -118,8 +124,8 @@ class Test_DB extends \Test\TestCase { array('user' => 'test', 'type' => 'contact', 'category' => 'School', 'expectedResult' => 1), ); - foreach($categoryentries as $entry) { - $result = OC_DB::insertIfNotExist('*PREFIX*'.$this->table3, + foreach($categoryEntries as $entry) { + $result = \OCP\DB::insertIfNotExist('*PREFIX*'.$this->table3, array( 'uid' => $entry['user'], 'type' => $entry['type'], @@ -135,14 +141,14 @@ class Test_DB extends \Test\TestCase { } public function testInsertIfNotExistNull() { - $categoryentries = array( + $categoryEntries = array( array('addressbookid' => 123, 'fullname' => null, 'expectedResult' => 1), array('addressbookid' => 123, 'fullname' => null, 'expectedResult' => 0), array('addressbookid' => 123, 'fullname' => 'test', 'expectedResult' => 1), ); - foreach($categoryentries as $entry) { - $result = OC_DB::insertIfNotExist('*PREFIX*'.$this->table2, + foreach($categoryEntries as $entry) { + $result = \OCP\DB::insertIfNotExist('*PREFIX*'.$this->table2, array( 'addressbookid' => $entry['addressbookid'], 'fullname' => $entry['fullname'], @@ -156,14 +162,14 @@ class Test_DB extends \Test\TestCase { $this->assertEquals(2, count($result->fetchAll())); } - public function testinsertIfNotExistDontOverwrite() { - $fullname = 'fullname test'; + public function testInsertIfNotExistDonTOverwrite() { + $fullName = 'fullname test'; $uri = 'uri_1'; $carddata = 'This is a vCard'; // Normal test to have same known data inserted. $query = OC_DB::prepare('INSERT INTO `*PREFIX*'.$this->table2.'` (`fullname`, `uri`, `carddata`) VALUES (?, ?, ?)'); - $result = $query->execute(array($fullname, $uri, $carddata)); + $result = $query->execute(array($fullName, $uri, $carddata)); $this->assertEquals(1, $result); $query = OC_DB::prepare('SELECT `fullname`, `uri`, `carddata` FROM `*PREFIX*'.$this->table2.'` WHERE `uri` = ?'); $result = $query->execute(array($uri)); @@ -174,9 +180,9 @@ class Test_DB extends \Test\TestCase { $this->assertEquals($carddata, $rowset[0]['carddata']); // Try to insert a new row - $result = OC_DB::insertIfNotExist('*PREFIX*'.$this->table2, + $result = \OCP\DB::insertIfNotExist('*PREFIX*'.$this->table2, array( - 'fullname' => $fullname, + 'fullname' => $fullName, 'uri' => $uri, )); $this->assertEquals(0, $result); @@ -192,6 +198,57 @@ class Test_DB extends \Test\TestCase { $this->assertEquals($carddata, $rowset[0]['carddata']); } + public function testInsertIfNotExistsViolating() { + $result = \OCP\DB::insertIfNotExist('*PREFIX*'.$this->table5, + array( + 'storage' => 1, + 'path_hash' => md5('welcome.txt'), + 'etag' => $this->getUniqueID() + )); + $this->assertEquals(1, $result); + + $result = \OCP\DB::insertIfNotExist('*PREFIX*'.$this->table5, + array( + 'storage' => 1, + 'path_hash' => md5('welcome.txt'), + 'etag' => $this->getUniqueID() + ),['storage', 'path_hash']); + + $this->assertEquals(0, $result); + } + + public function insertIfNotExistsViolatingThrows() { + return [ + [null], + [['etag']], + ]; + } + + /** + * @dataProvider insertIfNotExistsViolatingThrows + * @expectedException \Doctrine\DBAL\Exception\UniqueConstraintViolationException + * + * @param array $compareKeys + */ + public function testInsertIfNotExistsViolatingThrows($compareKeys) { + $result = \OCP\DB::insertIfNotExist('*PREFIX*'.$this->table5, + array( + 'storage' => 1, + 'path_hash' => md5('welcome.txt'), + 'etag' => $this->getUniqueID() + )); + $this->assertEquals(1, $result); + + $result = \OCP\DB::insertIfNotExist('*PREFIX*'.$this->table5, + array( + 'storage' => 1, + 'path_hash' => md5('welcome.txt'), + 'etag' => $this->getUniqueID() + ), $compareKeys); + + $this->assertEquals(0, $result); + } + public function testUtf8Data() { $table = "*PREFIX*{$this->table2}"; $expected = "ะ‹รถ้›™ๅ–œ\xE2\x80\xA2";