From 643c8d308847197550480507120b5f0f34f8fc80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 20 Jun 2013 14:46:22 +0200 Subject: [PATCH 1/6] make PDOStatementWrapper return number of updated rows on INSERT, UPDATE or DELETE queries, introduces isManipulation() to guess type of query --- lib/db.php | 80 ++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 65 insertions(+), 15 deletions(-) diff --git a/lib/db.php b/lib/db.php index a6b81aaba6..984c2bcf14 100644 --- a/lib/db.php +++ b/lib/db.php @@ -330,7 +330,7 @@ class OC_DB { * * SQL query via MDB2 prepare(), needs to be execute()'d! */ - static public function prepare( $query , $limit=null, $offset=null ) { + static public function prepare( $query , $limit = null, $offset = null, $isManipulation = null) { if (!is_null($limit) && $limit != -1) { if (self::$backend == self::BACKEND_MDB2) { @@ -367,12 +367,23 @@ class OC_DB { OC_Log::write('core', 'DB prepare : '.$query, OC_Log::DEBUG); } self::connect(); + + if ($isManipulation === null) { + //try to guess, so we return the number of rows on manipulations + $isManipulation = self::isManipulation($query); + } + // return the result if(self::$backend==self::BACKEND_MDB2) { - $result = self::$connection->prepare( $query ); + // differentiate between query and manipulation + if ($isManipulation) { + $result = self::$connection->prepare( $query, null, MDB2_PREPARE_MANIP ); + } else { + $result = self::$connection->prepare( $query, null, MDB2_PREPARE_RESULT ); + } // Die if we have an error (error means: bad query, not 0 results!) - if( PEAR::isError($result)) { + if( self::isError($result)) { throw new DatabaseException($result->getMessage(), $query); } }else{ @@ -381,7 +392,12 @@ class OC_DB { }catch(PDOException $e) { throw new DatabaseException($e->getMessage(), $query); } - $result=new PDOStatementWrapper($result); + // differentiate between query and manipulation + if ($isManipulation) { + $result=new PDOStatementWrapper($result, true); + } else { + $result=new PDOStatementWrapper($result, false); + } } if ((is_null($limit) || $limit == -1) and self::$cachingEnabled ) { $type = OC_Config::getValue( "dbtype", "sqlite" ); @@ -391,7 +407,33 @@ class OC_DB { } return $result; } - + + /** + * tries to guess the type of statement based on the first 10 characters + * the current check allows some whitespace but does not work with IF EXISTS or other more complex statements + * + * @param string $sql + */ + static public function isManipulation( $sql ) { + $selectOccurence = stripos ($sql, "SELECT"); + if ($selectOccurence !== false && $selectOccurence < 10) { + return false; + } + $insertOccurence = stripos ($sql, "INSERT"); + if ($insertOccurence !== false && $insertOccurence < 10) { + return true; + } + $updateOccurence = stripos ($sql, "UPDATE"); + if ($updateOccurence !== false && $updateOccurence < 10) { + return true; + } + $deleteOccurance = stripos ($sql, "DELETE"); + if ($deleteOccurance !== false && $deleteOccurance < 10) { + return true; + } + return false; + } + /** * @brief execute a prepared statement, on error write log and throw exception * @param mixed $stmt PDOStatementWrapper | MDB2_Statement_Common , @@ -718,6 +760,9 @@ class OC_DB { } catch(PDOException $e) { OC_Template::printExceptionErrorPage( $e ); } + if ($result === 0) { + return true; + } return $result; } @@ -919,7 +964,7 @@ class OC_DB { * @return bool */ public static function isError($result) { - if(!$result) { + if(self::$backend==self::BACKEND_PDO and $result === false) { return true; }elseif(self::$backend==self::BACKEND_MDB2 and PEAR::isError($result)) { return true; @@ -997,11 +1042,13 @@ class PDOStatementWrapper{ /** * @var PDOStatement */ - private $statement=null; - private $lastArguments=array(); + private $statement = null; + private $isManipulation = false; + private $lastArguments = array(); - public function __construct($statement) { - $this->statement=$statement; + public function __construct($statement, $isManipulation = false) { + $this->statement = $statement; + $this->isManipulation = $isManipulation; } /** @@ -1023,16 +1070,19 @@ class PDOStatementWrapper{ $input = $this->tryFixSubstringLastArgumentDataForMSSQL($input); } - $result=$this->statement->execute($input); + $result = $this->statement->execute($input); } else { - $result=$this->statement->execute(); + $result = $this->statement->execute(); } - if ($result) { - return $this; - } else { + if ($result === false) { return false; } + if ($this->isManipulation) { + return $this->statement->rowCount(); + } else { + return $this; + } } private function tryFixSubstringLastArgumentDataForMSSQL($input) { From 88fc410c1936510b84497b677248cd20db9fd87f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 20 Jun 2013 14:47:12 +0200 Subject: [PATCH 2/6] fix numRows usage in user_ldap --- apps/user_ldap/lib/access.php | 6 ++---- apps/user_ldap/lib/helper.php | 6 +++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 04f73cf01f..6f6b8d0f01 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -578,14 +578,12 @@ abstract class Access { '); //feed the DB - $res = $insert->execute(array($dn, $ocname, $this->getUUID($dn), $dn, $ocname)); + $insRows = $insert->execute(array($dn, $ocname, $this->getUUID($dn), $dn, $ocname)); - if(\OCP\DB::isError($res)) { + if(\OCP\DB::isError($insRows)) { return false; } - $insRows = $res->numRows(); - if($insRows === 0) { return false; } diff --git a/apps/user_ldap/lib/helper.php b/apps/user_ldap/lib/helper.php index 10ed40ebd6..f65f466789 100644 --- a/apps/user_ldap/lib/helper.php +++ b/apps/user_ldap/lib/helper.php @@ -90,13 +90,13 @@ class Helper { AND `appid` = \'user_ldap\' AND `configkey` NOT IN (\'enabled\', \'installed_version\', \'types\', \'bgjUpdateGroupsLastRun\') '); - $res = $query->execute(array($prefix.'%')); + $delRows = $query->execute(array($prefix.'%')); - if(\OCP\DB::isError($res)) { + if(\OCP\DB::isError($delRows)) { return false; } - if($res->numRows() === 0) { + if($delRows === 0) { return false; } From c223bee6df33b3208dbb5a7c46dbeb98eccf10ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 20 Jun 2013 14:47:42 +0200 Subject: [PATCH 3/6] fix numRows usage in core lib --- lib/connector/sabre/locks.php | 2 +- lib/vcategories.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/connector/sabre/locks.php b/lib/connector/sabre/locks.php index cbc495dec1..e057b05994 100644 --- a/lib/connector/sabre/locks.php +++ b/lib/connector/sabre/locks.php @@ -178,7 +178,7 @@ class OC_Connector_Sabre_Locks extends Sabre_DAV_Locks_Backend_Abstract { $sql = 'DELETE FROM `*PREFIX*locks` WHERE `userid` = ? AND `uri` = ? AND `token` = ?'; $result = OC_DB::executeAudited( $sql, array(OC_User::getUser(), $uri, $lockInfo->token)); - return $result->numRows() === 1; + return $result === 1; } diff --git a/lib/vcategories.php b/lib/vcategories.php index 7bac6e7d4e..8403695835 100644 --- a/lib/vcategories.php +++ b/lib/vcategories.php @@ -125,7 +125,7 @@ class OC_VCategories { OC_Log::write('core', __METHOD__. 'DB error: ' . OC_DB::getErrorMessage($result), OC_Log::ERROR); return false; } - return ($result->numRows() == 0); + return ($result->numRows() === 0); } catch(Exception $e) { OCP\Util::writeLog('core', __METHOD__.', exception: '.$e->getMessage(), OCP\Util::ERROR); From c79f7f4f3cb460f890fccb5a7de7d62ea0a5fc15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 20 Jun 2013 14:49:10 +0200 Subject: [PATCH 4/6] fix numRows usage in files_encryption --- apps/files_encryption/lib/util.php | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php index e8e53859bd..b3de85254e 100644 --- a/apps/files_encryption/lib/util.php +++ b/apps/files_encryption/lib/util.php @@ -362,17 +362,7 @@ class Util { } - $query = \OCP\DB::prepare($sql); - - if ($query->execute($args)) { - - return true; - - } else { - - return false; - - } + return is_numeric(\OC_DB::executeAudited($sql, $args)); } @@ -1063,8 +1053,7 @@ class Util { $sql = 'UPDATE `*PREFIX*encryption` SET `migration_status` = ? WHERE `uid` = ? and `migration_status` = ?'; $args = array(self::MIGRATION_IN_PROGRESS, $this->userId, self::MIGRATION_OPEN); $query = \OCP\DB::prepare($sql); - $result = $query->execute($args); - $manipulatedRows = $result->numRows(); + $manipulatedRows = $query->execute($args); if ($manipulatedRows === 1) { $return = true; @@ -1087,8 +1076,7 @@ class Util { $sql = 'UPDATE `*PREFIX*encryption` SET `migration_status` = ? WHERE `uid` = ? and `migration_status` = ?'; $args = array(self::MIGRATION_COMPLETED, $this->userId, self::MIGRATION_IN_PROGRESS); $query = \OCP\DB::prepare($sql); - $result = $query->execute($args); - $manipulatedRows = $result->numRows(); + $manipulatedRows = $query->execute($args); if ($manipulatedRows === 1) { $return = true; From 1b97c186b4f2946753123fb73314597b818aea70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 20 Jun 2013 15:38:02 +0200 Subject: [PATCH 5/6] use assertEquals number of rows in db tests --- tests/lib/db.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/lib/db.php b/tests/lib/db.php index afbdb413c3..0ba7d5d4b0 100644 --- a/tests/lib/db.php +++ b/tests/lib/db.php @@ -40,7 +40,7 @@ class Test_DB extends PHPUnit_Framework_TestCase { $this->assertFalse((bool)$row); //PDO returns false, MDB2 returns null $query = OC_DB::prepare('INSERT INTO `*PREFIX*'.$this->table2.'` (`fullname`,`uri`) VALUES (?,?)'); $result = $query->execute(array('fullname test', 'uri_1')); - $this->assertTrue((bool)$result); + $this->assertEquals('1', $result); $query = OC_DB::prepare('SELECT `fullname`,`uri` FROM `*PREFIX*'.$this->table2.'` WHERE `uri` = ?'); $result = $query->execute(array('uri_1')); $this->assertTrue((bool)$result); @@ -57,7 +57,7 @@ class Test_DB extends PHPUnit_Framework_TestCase { public function testNOW() { $query = OC_DB::prepare('INSERT INTO `*PREFIX*'.$this->table2.'` (`fullname`,`uri`) VALUES (NOW(),?)'); $result = $query->execute(array('uri_2')); - $this->assertTrue((bool)$result); + $this->assertEquals('1', $result); $query = OC_DB::prepare('SELECT `fullname`,`uri` FROM `*PREFIX*'.$this->table2.'` WHERE `uri` = ?'); $result = $query->execute(array('uri_2')); $this->assertTrue((bool)$result); @@ -66,7 +66,7 @@ class Test_DB extends PHPUnit_Framework_TestCase { public function testUNIX_TIMESTAMP() { $query = OC_DB::prepare('INSERT INTO `*PREFIX*'.$this->table2.'` (`fullname`,`uri`) VALUES (UNIX_TIMESTAMP(),?)'); $result = $query->execute(array('uri_3')); - $this->assertTrue((bool)$result); + $this->assertEquals('1', $result); $query = OC_DB::prepare('SELECT `fullname`,`uri` FROM `*PREFIX*'.$this->table2.'` WHERE `uri` = ?'); $result = $query->execute(array('uri_3')); $this->assertTrue((bool)$result); @@ -105,7 +105,7 @@ class Test_DB extends PHPUnit_Framework_TestCase { // 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)); - $this->assertTrue((bool)$result); + $this->assertEquals('1', $result); $query = OC_DB::prepare('SELECT `fullname`, `uri`, `carddata` FROM `*PREFIX*'.$this->table2.'` WHERE `uri` = ?'); $result = $query->execute(array($uri)); $this->assertTrue((bool)$result); From 45c897acf34151672d68ee767ff15ab010676335 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 27 Jun 2013 13:13:49 +0200 Subject: [PATCH 6/6] one if less --- lib/db.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/db.php b/lib/db.php index 984c2bcf14..fecc622d96 100644 --- a/lib/db.php +++ b/lib/db.php @@ -326,6 +326,7 @@ class OC_DB { * @param string $query Query string * @param int $limit * @param int $offset + * @param bool $isManipulation * @return MDB2_Statement_Common prepared SQL query * * SQL query via MDB2 prepare(), needs to be execute()'d! @@ -393,11 +394,7 @@ class OC_DB { throw new DatabaseException($e->getMessage(), $query); } // differentiate between query and manipulation - if ($isManipulation) { - $result=new PDOStatementWrapper($result, true); - } else { - $result=new PDOStatementWrapper($result, false); - } + $result = new PDOStatementWrapper($result, $isManipulation); } if ((is_null($limit) || $limit == -1) and self::$cachingEnabled ) { $type = OC_Config::getValue( "dbtype", "sqlite" );