rdbms: make affectedRows() work more consistently

* Update replace()/upsert() to combine the affected row
  count for the non-native case
* Also make replace() atomic in the non-native case,
  similar to how upsert() already works

Change-Id: I6c9bcba54eca6bcf4a93a9b230aaedf7f36aa877

diff --git a/includes/db/DatabaseOracle.php b/includes/db/DatabaseOracle.php
index e2feb1f..90fabaf 100644 (file)
--- a/includes/db/DatabaseOracle.php
+++ b/includes/db/DatabaseOracle.php
@@ -354,7 +354,7 @@ class DatabaseOracle extends Database {
                return $e['code'];
        }
 
-       function affectedRows() {
+       protected function fetchAffectedRowCount() {
                return $this->mAffectedRows;
        }
 

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 6d1cff7..f3877fb 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -126,6 +126,8 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        protected $delimiter = ';';
        /** @var DatabaseDomain */
        protected $currentDomain;
+       /** @var integer|null Rows affected by the last query to query() or its CRUD wrappers */
+       protected $affectedRowCount;
 
        /**
         * Either 1 if a transaction is active or 0 otherwise.
@@ -1002,8 +1004,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        /**
-        * Helper method for query() that handles profiling and logging and sends
-        * the query to doQuery()
+        * Wrapper for query() that also handles profiling, logging, and affected row count updates
         *
         * @param string $sql Original SQL query
         * @param string $commentedSql SQL query with debugging/trace comment
@@ -1029,7 +1030,9 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                if ( $this->profiler ) {
                        call_user_func( [ $this->profiler, 'profileIn' ], $queryProf );
                }
+               $this->affectedRowCount = null;
                $ret = $this->doQuery( $commentedSql );
+               $this->affectedRowCount = $this->affectedRows();
                if ( $this->profiler ) {
                        call_user_func( [ $this->profiler, 'profileOut' ], $queryProf );
                }
@@ -2247,7 +2250,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        $rows = [ $rows ];
                }
 
-               // @FIXME: this is not atomic, but a trx would break affectedRows()
+               $affectedRowCount = 0;
                foreach ( $rows as $row ) {
                        // Delete rows which collide with this one
                        $indexWhereClauses = [];
@@ -2272,11 +2275,15 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 
                        if ( $indexWhereClauses ) {
                                $this->delete( $table, $this->makeList( $indexWhereClauses, LIST_OR ), $fname );
+                               $affectedRowCount += $this->affectedRows();
                        }
 
                        // Now insert the row
                        $this->insert( $table, $row, $fname );
+                       $affectedRowCount += $this->affectedRows();
                }
+
+               $this->affectedRowCount = $affectedRowCount;
        }
 
        /**
@@ -2341,6 +2348,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        $where = false;
                }
 
+               $affectedRowCount = 0;
                $useTrx = !$this->mTrxLevel;
                if ( $useTrx ) {
                        $this->begin( $fname, self::TRANSACTION_INTERNAL );
@@ -2349,11 +2357,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        # Update any existing conflicting row(s)
                        if ( $where !== false ) {
                                $ok = $this->update( $table, $set, $where, $fname );
+                               $affectedRowCount += $this->affectedRows();
                        } else {
                                $ok = true;
                        }
                        # Now insert any non-conflicting row(s)
                        $ok = $this->insert( $table, $rows, $fname, [ 'IGNORE' ] ) && $ok;
+                       $affectedRowCount += $this->affectedRows();
                } catch ( Exception $e ) {
                        if ( $useTrx ) {
                                $this->rollback( $fname, self::FLUSHING_INTERNAL );
@@ -2363,6 +2373,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                if ( $useTrx ) {
                        $this->commit( $fname, self::FLUSHING_INTERNAL );
                }
+               $this->affectedRowCount = $affectedRowCount;
 
                return $ok;
        }
@@ -3181,6 +3192,17 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                }
        }
 
+       public function affectedRows() {
+               return ( $this->affectedRowCount === null )
+                       ? $this->fetchAffectedRowCount() // default to driver value
+                       : $this->affectedRowCount;
+       }
+
+       /**
+        * @return int Number of retrieved rows according to the driver
+        */
+       abstract protected function fetchAffectedRowCount();
+
        /**
         * Take the result from a query, and wrap it in a ResultWrapper if
         * necessary. Boolean values are passed through as is, to indicate success

diff --git a/includes/libs/rdbms/database/DatabaseMssql.php b/includes/libs/rdbms/database/DatabaseMssql.php
index 53beb65..832ed9e 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseMssql.php
+++ b/includes/libs/rdbms/database/DatabaseMssql.php
@@ -353,7 +353,7 @@ class DatabaseMssql extends Database {
        /**
         * @return int
         */
-       public function affectedRows() {
+       protected function fetchAffectedRowCount() {
                return $this->mAffectedRows;
        }
 

diff --git a/includes/libs/rdbms/database/DatabaseMysqli.php b/includes/libs/rdbms/database/DatabaseMysqli.php
index c1a5698..09ea66c 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseMysqli.php
+++ b/includes/libs/rdbms/database/DatabaseMysqli.php
@@ -172,7 +172,7 @@ class DatabaseMysqli extends DatabaseMysqlBase {
        /**
         * @return int
         */
-       function affectedRows() {
+       protected function fetchAffectedRowCount() {
                $conn = $this->getBindingHandle();
 
                return $conn->affected_rows;

diff --git a/includes/libs/rdbms/database/DatabasePostgres.php b/includes/libs/rdbms/database/DatabasePostgres.php
index 8c21d72..5bf845b 100644 (file)
--- a/includes/libs/rdbms/database/DatabasePostgres.php
+++ b/includes/libs/rdbms/database/DatabasePostgres.php
@@ -384,7 +384,7 @@ class DatabasePostgres extends Database {
                }
        }
 
-       public function affectedRows() {
+       protected function fetchAffectedRowCount() {
                if ( !is_null( $this->mAffectedRows ) ) {
                        // Forced result for simulated queries
                        return $this->mAffectedRows;

diff --git a/includes/libs/rdbms/database/DatabaseSqlite.php b/includes/libs/rdbms/database/DatabaseSqlite.php
index 3bdcd65..01772cf 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseSqlite.php
+++ b/includes/libs/rdbms/database/DatabaseSqlite.php
@@ -496,7 +496,7 @@ class DatabaseSqlite extends Database {
        /**
         * @return int
         */
-       function affectedRows() {
+       protected function fetchAffectedRowCount() {
                return $this->mAffectedRows;
        }
 

diff --git a/tests/phpunit/includes/db/DatabaseTestHelper.php b/tests/phpunit/includes/db/DatabaseTestHelper.php
index d19d998..fa9898d 100644 (file)
--- a/tests/phpunit/includes/db/DatabaseTestHelper.php
+++ b/tests/phpunit/includes/db/DatabaseTestHelper.php
@@ -174,7 +174,7 @@ class DatabaseTestHelper extends Database {
                return false;
        }
 
-       function affectedRows() {
+       function fetchAffectedRowCount() {
                return -1;
        }
 

diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
index a23a2a0..caf1281 100644 (file)
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php
@@ -53,6 +53,9 @@ class FakeDatabaseMysqlBase extends DatabaseMysqlBase {
        protected function doQuery( $sql ) {
        }
 
+       protected function fetchAffectedRowCount() {
+       }
+
        // From DatabaseMysqli
        protected function mysqlConnect( $realServer ) {
        }

diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
index 4a0a7e7..e131506 100644 (file)
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
@@ -322,7 +322,7 @@ class DatabaseTest extends PHPUnit_Framework_TestCase {
         */
        private function getMockDB( $methods = [] ) {
                static $abstractMethods = [
-                       'affectedRows',
+                       'fetchAffectedRowCount',
                        'closeConnection',
                        'dataSeek',
                        'doQuery',