Fix edge case in onTransactionIdle()

If a callback set DBO_TRX, make sure it gets unset if that
is not what the DB was set to before the callbacks ran.

Change-Id: I79b649de02e821494d7f88e8979764ec8a9d0c43

diff --git a/includes/db/Database.php b/includes/db/Database.php
index 68f94b6..dea4a59 100644 (file)
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -3308,7 +3308,11 @@ abstract class DatabaseBase implements IDatabase {
                                        list( $phpCallback ) = $callback;
                                        $this->clearFlag( DBO_TRX ); // make each query its own transaction
                                        call_user_func( $phpCallback );
-                                       $this->setFlag( $autoTrx ? DBO_TRX : 0 ); // restore automatic begin()
+                                       if ( $autoTrx ) {
+                                               $this->setFlag( DBO_TRX ); // restore automatic begin()
+                                       } else {
+                                               $this->clearFlag( DBO_TRX ); // restore auto-commit
+                                       }
                                } catch ( Exception $e ) {
                                        if ( $ePrior ) {
                                                MWExceptionHandler::logException( $ePrior );

diff --git a/tests/phpunit/includes/db/DatabaseTest.php b/tests/phpunit/includes/db/DatabaseTest.php
index 7e70439..5f2a6fe 100644 (file)
--- a/tests/phpunit/includes/db/DatabaseTest.php
+++ b/tests/phpunit/includes/db/DatabaseTest.php
@@ -234,4 +234,30 @@ class DatabaseTest extends MediaWikiTestCase {
                $this->assertFalse( $this->db->tableExists( 'foobarbaz' ) );
                $this->assertInternalType( 'int', $res->numRows() );
        }
+
+       public function testTransactionIdle() {
+               $db = $this->db;
+
+               $db->setFlag( DBO_TRX );
+               $flagSet = null;
+               $db->onTransactionIdle( function() use ( $db, &$flagSet ) {
+                       $flagSet = $db->getFlag( DBO_TRX );
+               } );
+               $this->assertFalse( $flagSet, 'DBO_TRX off in callback' );
+               $this->assertTrue( $db->getFlag( DBO_TRX ), 'DBO_TRX restored to default' );
+
+               $db->clearFlag( DBO_TRX );
+               $flagSet = null;
+               $db->onTransactionIdle( function() use ( $db, &$flagSet ) {
+                       $flagSet = $db->getFlag( DBO_TRX );
+               } );
+               $this->assertFalse( $flagSet, 'DBO_TRX off in callback' );
+               $this->assertFalse( $db->getFlag( DBO_TRX ), 'DBO_TRX restored to default' );
+
+               $db->clearFlag( DBO_TRX );
+               $db->onTransactionIdle( function() use ( $db ) {
+                       $db->setFlag( DBO_TRX );
+               } );
+               $this->assertFalse( $db->getFlag( DBO_TRX ), 'DBO_TRX restored to default' );
+       }
 }