rdbms: make IDatabase::onTransaction* methods pass the DB handle for convenience

Change-Id: Ia45a26830d62326b103593268fbf34c907783c90

diff --git a/includes/FileDeleteForm.php b/includes/FileDeleteForm.php
index 783de1c..898005e 100644 (file)
--- a/includes/FileDeleteForm.php
+++ b/includes/FileDeleteForm.php
@@ -212,7 +212,7 @@ class FileDeleteForm {
                                                $logEntry->setTags( $tags );
                                                $logid = $logEntry->insert();
                                                $dbw->onTransactionPreCommitOrIdle(
-                                                       function () use ( $dbw, $logEntry, $logid ) {
+                                                       function () use ( $logEntry, $logid ) {
                                                                $logEntry->publish( $logid );
                                                        },
                                                        __METHOD__

diff --git a/includes/jobqueue/JobQueueDB.php b/includes/jobqueue/JobQueueDB.php
index f01ba63..c3193c3 100644 (file)
--- a/includes/jobqueue/JobQueueDB.php
+++ b/includes/jobqueue/JobQueueDB.php
@@ -196,7 +196,7 @@ class JobQueueDB extends JobQueue {
                // errors that bubble up will rollback the main commit round.
                $fname = __METHOD__;
                $dbw->onTransactionPreCommitOrIdle(
-                       function () use ( $dbw, $jobs, $flags, $fname ) {
+                       function ( IDatabase $dbw ) use ( $jobs, $flags, $fname ) {
                                $this->doBatchPushInternal( $dbw, $jobs, $flags, $fname );
                        },
                        $fname
@@ -508,7 +508,7 @@ class JobQueueDB extends JobQueue {
                $dbw = $this->getMasterDB();
                $cache = $this->dupCache;
                $dbw->onTransactionIdle(
-                       function () use ( $cache, $params, $key, $dbw ) {
+                       function () use ( $cache, $params, $key ) {
                                $timestamp = $cache->get( $key ); // current last timestamp of this job
                                if ( $timestamp && $timestamp >= $params['rootJobTimestamp'] ) {
                                        return true; // a newer version of this root job was enqueued

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 5549f28..189eaf2 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -3322,7 +3322,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        // No transaction is active nor will start implicitly, so make one for this callback
                        $this->startAtomic( __METHOD__, self::ATOMIC_CANCELABLE );
                        try {
-                               call_user_func( $callback );
+                               call_user_func( $callback, $this );
                                $this->endAtomic( __METHOD__ );
                        } catch ( Exception $e ) {
                                $this->cancelAtomic( __METHOD__ );
@@ -3391,7 +3391,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        if ( in_array( $entry[2], $sectionIds, true ) ) {
                                $callback = $entry[0];
                                $this->trxEndCallbacks[$key][0] = function () use ( $callback ) {
-                                       return $callback( self::TRIGGER_ROLLBACK );
+                                       return $callback( self::TRIGGER_ROLLBACK, $this );
                                };
                        }
                }
@@ -3445,7 +3445,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                                try {
                                        list( $phpCallback ) = $callback;
                                        $this->clearFlag( self::DBO_TRX ); // make each query its own transaction
-                                       call_user_func_array( $phpCallback, [ $trigger ] );
+                                       call_user_func( $phpCallback, $trigger, $this );
                                        if ( $autoTrx ) {
                                                $this->setFlag( self::DBO_TRX ); // restore automatic begin()
                                        } else {
@@ -3484,7 +3484,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        foreach ( $callbacks as $callback ) {
                                try {
                                        list( $phpCallback ) = $callback;
-                                       call_user_func( $phpCallback );
+                                       call_user_func( $phpCallback, $this );
                                } catch ( Exception $ex ) {
                                        call_user_func( $this->errorLogger, $ex );
                                        $e = $e ?: $ex;

diff --git a/includes/libs/rdbms/database/IDatabase.php b/includes/libs/rdbms/database/IDatabase.php
index 675ba7f..6b3efa2 100644 (file)
--- a/includes/libs/rdbms/database/IDatabase.php
+++ b/includes/libs/rdbms/database/IDatabase.php
@@ -1480,8 +1480,9 @@ interface IDatabase {
         *
         * @note: do not assume that *other* IDatabase instances will be AUTOCOMMIT mode
         *
-        * The callback takes one argument:
+        * The callback takes the following arguments:
         *   - How the transaction ended (IDatabase::TRIGGER_COMMIT or IDatabase::TRIGGER_ROLLBACK)
+        *   - This IDatabase instance (since 1.32)
         *
         * @param callable $callback
         * @param string $fname Caller name
@@ -1511,8 +1512,9 @@ interface IDatabase {
         *
         * @note: do not assume that *other* IDatabase instances will be AUTOCOMMIT mode
         *
-        * The callback takes one argument:
+        * The callback takes the following arguments:
         *   - How the transaction ended (IDatabase::TRIGGER_COMMIT or IDatabase::TRIGGER_IDLE)
+        *   - This IDatabase instance (since 1.32)
         *
         * @param callable $callback
         * @param string $fname Caller name
@@ -1536,6 +1538,9 @@ interface IDatabase {
         *
         * Updates will execute in the order they were enqueued.
         *
+        * The callback takes the one argument:
+        *   - This IDatabase instance (since 1.32)
+        *
         * @param callable $callback
         * @param string $fname Caller name
         * @since 1.22

diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php
index f6e2bd0..cbce884 100644 (file)
--- a/includes/page/WikiPage.php
+++ b/includes/page/WikiPage.php
@@ -2978,7 +2978,7 @@ class WikiPage implements Page, IDBAccessObject {
                $logid = $logEntry->insert();
 
                $dbw->onTransactionPreCommitOrIdle(
-                       function () use ( $dbw, $logEntry, $logid ) {
+                       function () use ( $logEntry, $logid ) {
                                // T58776: avoid deadlocks (especially from FileDeleteForm)
                                $logEntry->publish( $logid );
                        },

diff --git a/includes/profiler/output/ProfilerOutputDb.php b/includes/profiler/output/ProfilerOutputDb.php
index 28dc2cc..1c3d479 100644 (file)
--- a/includes/profiler/output/ProfilerOutputDb.php
+++ b/includes/profiler/output/ProfilerOutputDb.php
@@ -21,6 +21,7 @@
  * @ingroup Profiler
  */
 
+use Wikimedia\Rdbms\Database;
 use Wikimedia\Rdbms\DBError;
 
 /**
@@ -55,7 +56,7 @@ class ProfilerOutputDb extends ProfilerOutput {
                }
 
                $fname = __METHOD__;
-               $dbw->onTransactionIdle( function () use ( $stats, $dbw, $fname ) {
+               $dbw->onTransactionIdle( function ( Database $dbw ) use ( $stats, $fname ) {
                        $pfhost = $this->perHost ? wfHostname() : '';
                        // Sqlite: avoid excess b-tree rebuilds (mostly for non-WAL mode)
                        // non-Sqlite: lower contention with small transactions

diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
index 3d15b03..4596c76 100644 (file)
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php
@@ -1434,6 +1434,9 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
                        IDatabase::TRIGGER_COMMIT => 'tCommit',
                        IDatabase::TRIGGER_ROLLBACK => 'tRollback'
                ];
+               $pcCallback = function ( IDatabase $db ) use ( $fname ) {
+                       $this->database->query( "SELECT 0", $fname );
+               };
                $callback1 = function ( $trigger = '-' ) use ( $fname, $triggerMap ) {
                        $this->database->query( "SELECT 1, {$triggerMap[$trigger]} AS t", $fname );
                };
@@ -1445,7 +1448,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
                };
 
                $this->database->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE );
-               $this->database->onTransactionPreCommitOrIdle( $callback1, __METHOD__ );
+               $this->database->onTransactionPreCommitOrIdle( $pcCallback, __METHOD__ );
                $this->database->cancelAtomic( __METHOD__ );
                $this->assertLastSql( 'BEGIN; ROLLBACK' );
 
@@ -1460,18 +1463,18 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
                $this->assertLastSql( 'BEGIN; ROLLBACK; SELECT 1, tRollback AS t' );
 
                $this->database->startAtomic( __METHOD__ . '_outer' );
-               $this->database->onTransactionPreCommitOrIdle( $callback1, __METHOD__ );
+               $this->database->onTransactionPreCommitOrIdle( $pcCallback, __METHOD__ );
                $this->database->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE );
-               $this->database->onTransactionPreCommitOrIdle( $callback2, __METHOD__ );
+               $this->database->onTransactionPreCommitOrIdle( $pcCallback, __METHOD__ );
                $this->database->cancelAtomic( __METHOD__ );
-               $this->database->onTransactionPreCommitOrIdle( $callback3, __METHOD__ );
+               $this->database->onTransactionPreCommitOrIdle( $pcCallback, __METHOD__ );
                $this->database->endAtomic( __METHOD__ . '_outer' );
                $this->assertLastSql( implode( "; ", [
                        'BEGIN',
                        'SAVEPOINT wikimedia_rdbms_atomic1',
                        'ROLLBACK TO SAVEPOINT wikimedia_rdbms_atomic1',
-                       'SELECT 1, - AS t',
-                       'SELECT 3, - AS t',
+                       'SELECT 0',
+                       'SELECT 0',
                        'COMMIT'
                ] ) );
 

diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
index e328cba..3335a2b 100644 (file)
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
@@ -180,7 +180,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
                $db->clearFlag( DBO_TRX );
                $called = false;
                $flagSet = null;
-               $callback = function () use ( $db, &$flagSet, &$called ) {
+               $callback = function ( $trigger, IDatabase $db ) use ( &$flagSet, &$called ) {
                        $called = true;
                        $flagSet = $db->getFlag( DBO_TRX );
                };
@@ -203,7 +203,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
 
                $db->clearFlag( DBO_TRX );
                $db->onTransactionIdle(
-                       function () use ( $db ) {
+                       function ( $trigger, IDatabase $db ) {
                                $db->setFlag( DBO_TRX );
                        },
                        __METHOD__
@@ -274,7 +274,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
 
                $called = false;
                $db->onTransactionPreCommitOrIdle(
-                       function () use ( &$called ) {
+                       function ( IDatabase $db ) use ( &$called ) {
                                $called = true;
                        },
                        __METHOD__
@@ -284,7 +284,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
                $db->begin( __METHOD__ );
                $called = false;
                $db->onTransactionPreCommitOrIdle(
-                       function () use ( &$called ) {
+                       function ( IDatabase $db ) use ( &$called ) {
                                $called = true;
                        },
                        __METHOD__
@@ -314,7 +314,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
                $this->assertFalse( $lb->hasMasterChanges() );
                $this->assertTrue( $db->getFlag( DBO_TRX ), 'DBO_TRX is set' );
                $called = false;
-               $callback = function () use ( &$called ) {
+               $callback = function ( IDatabase $db ) use ( &$called ) {
                        $called = true;
                };
                $db->onTransactionPreCommitOrIdle( $callback, __METHOD__ );
@@ -352,7 +352,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
                $db->clearFlag( DBO_TRX );
                $db->begin( __METHOD__ );
                $called = false;
-               $db->onTransactionResolution( function () use ( $db, &$called ) {
+               $db->onTransactionResolution( function ( $trigger, IDatabase $db ) use ( &$called ) {
                        $called = true;
                        $db->setFlag( DBO_TRX );
                } );
@@ -363,7 +363,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
                $db->clearFlag( DBO_TRX );
                $db->begin( __METHOD__ );
                $called = false;
-               $db->onTransactionResolution( function () use ( $db, &$called ) {
+               $db->onTransactionResolution( function ( $trigger, IDatabase $db ) use ( &$called ) {
                        $called = true;
                        $db->setFlag( DBO_TRX );
                } );