From b6cd5421b915d3f74928d00f3fd3af34b7ebc2a6 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Tue, 8 May 2018 19:28:39 -0700 Subject: [PATCH] rdbms: rename onTransactionIdle() to onTransactionCommitOrIdle() This is clearer and is consistent with onTransactionPreCommitOrIdle() Change-Id: I3a34a0e9adea69ec55ed6ddfef47703e31e7c3b5 --- ...icationSecondaryAuthenticationProvider.php | 2 +- ...yPasswordPrimaryAuthenticationProvider.php | 4 ++-- includes/changes/RecentChange.php | 2 +- includes/jobqueue/JobQueueDB.php | 2 +- includes/libs/rdbms/database/DBConnRef.php | 4 ++++ includes/libs/rdbms/database/Database.php | 6 ++++- includes/libs/rdbms/database/IDatabase.php | 13 +++++++++- includes/profiler/output/ProfilerOutputDb.php | 2 +- .../phpunit/includes/db/LoadBalancerTest.php | 8 +++---- .../includes/deferred/DeferredUpdatesTest.php | 2 +- .../libs/rdbms/database/DatabaseSQLTest.php | 24 +++++++++---------- .../libs/rdbms/database/DatabaseTest.php | 16 ++++++------- 12 files changed, 52 insertions(+), 33 deletions(-) diff --git a/includes/auth/EmailNotificationSecondaryAuthenticationProvider.php b/includes/auth/EmailNotificationSecondaryAuthenticationProvider.php index a4855318b5..0878c34f42 100644 --- a/includes/auth/EmailNotificationSecondaryAuthenticationProvider.php +++ b/includes/auth/EmailNotificationSecondaryAuthenticationProvider.php @@ -51,7 +51,7 @@ class EmailNotificationSecondaryAuthenticationProvider && !$this->manager->getAuthenticationSessionData( 'no-email' ) ) { // TODO show 'confirmemail_oncreate'/'confirmemail_sendfailed' message - wfGetDB( DB_MASTER )->onTransactionIdle( + wfGetDB( DB_MASTER )->onTransactionCommitOrIdle( function () use ( $user ) { $user = $user->getInstanceForUpdate(); $status = $user->sendConfirmationMail(); diff --git a/includes/auth/TemporaryPasswordPrimaryAuthenticationProvider.php b/includes/auth/TemporaryPasswordPrimaryAuthenticationProvider.php index 4a2d0094eb..0ef13b34ec 100644 --- a/includes/auth/TemporaryPasswordPrimaryAuthenticationProvider.php +++ b/includes/auth/TemporaryPasswordPrimaryAuthenticationProvider.php @@ -314,7 +314,7 @@ class TemporaryPasswordPrimaryAuthenticationProvider if ( $sendMail ) { // Send email after DB commit - $dbw->onTransactionIdle( + $dbw->onTransactionCommitOrIdle( function () use ( $req ) { /** @var TemporaryPasswordAuthenticationRequest $req */ $this->sendPasswordResetEmail( $req ); @@ -388,7 +388,7 @@ class TemporaryPasswordPrimaryAuthenticationProvider if ( $mailpassword ) { // Send email after DB commit - wfGetDB( DB_MASTER )->onTransactionIdle( + wfGetDB( DB_MASTER )->onTransactionCommitOrIdle( function () use ( $user, $creator, $req ) { $this->sendNewAccountEmail( $user, $creator, $req->password ); }, diff --git a/includes/changes/RecentChange.php b/includes/changes/RecentChange.php index eea8af3617..cc9532e064 100644 --- a/includes/changes/RecentChange.php +++ b/includes/changes/RecentChange.php @@ -464,7 +464,7 @@ class RecentChange { ) { // @FIXME: This would be better as an extension hook // Send emails or email jobs once this row is safely committed - $dbw->onTransactionIdle( + $dbw->onTransactionCommitOrIdle( function () use ( $editor, $title ) { $enotif = new EmailNotification(); $enotif->notifyOnPageChange( diff --git a/includes/jobqueue/JobQueueDB.php b/includes/jobqueue/JobQueueDB.php index c3193c3ba7..c13f5396af 100644 --- a/includes/jobqueue/JobQueueDB.php +++ b/includes/jobqueue/JobQueueDB.php @@ -507,7 +507,7 @@ class JobQueueDB extends JobQueue { // jobs to become no-ops without any actual jobs that made them redundant. $dbw = $this->getMasterDB(); $cache = $this->dupCache; - $dbw->onTransactionIdle( + $dbw->onTransactionCommitOrIdle( function () use ( $cache, $params, $key ) { $timestamp = $cache->get( $key ); // current last timestamp of this job if ( $timestamp && $timestamp >= $params['rootJobTimestamp'] ) { diff --git a/includes/libs/rdbms/database/DBConnRef.php b/includes/libs/rdbms/database/DBConnRef.php index 3432bffe1a..9de16c4a46 100644 --- a/includes/libs/rdbms/database/DBConnRef.php +++ b/includes/libs/rdbms/database/DBConnRef.php @@ -487,6 +487,10 @@ class DBConnRef implements IDatabase { return $this->__call( __FUNCTION__, func_get_args() ); } + public function onTransactionCommitOrIdle( callable $callback, $fname = __METHOD__ ) { + return $this->__call( __FUNCTION__, func_get_args() ); + } + public function onTransactionIdle( callable $callback, $fname = __METHOD__ ) { return $this->__call( __FUNCTION__, func_get_args() ); } diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php index 1517bd931c..aeda5b9ce0 100644 --- a/includes/libs/rdbms/database/Database.php +++ b/includes/libs/rdbms/database/Database.php @@ -3292,7 +3292,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware $this->trxEndCallbacks[] = [ $callback, $fname, $this->currentAtomicSectionId() ]; } - final public function onTransactionIdle( callable $callback, $fname = __METHOD__ ) { + final public function onTransactionCommitOrIdle( callable $callback, $fname = __METHOD__ ) { if ( !$this->trxLevel && $this->getTransactionRoundId() ) { // Start an implicit transaction similar to how query() does $this->begin( __METHOD__, self::TRANSACTION_INTERNAL ); @@ -3305,6 +3305,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware } } + final public function onTransactionIdle( callable $callback, $fname = __METHOD__ ) { + $this->onTransactionCommitOrIdle( $callback, $fname ); + } + final public function onTransactionPreCommitOrIdle( callable $callback, $fname = __METHOD__ ) { if ( !$this->trxLevel && $this->getTransactionRoundId() ) { // Start an implicit transaction similar to how query() does diff --git a/includes/libs/rdbms/database/IDatabase.php b/includes/libs/rdbms/database/IDatabase.php index 43e97510e8..ca3fd52cf4 100644 --- a/includes/libs/rdbms/database/IDatabase.php +++ b/includes/libs/rdbms/database/IDatabase.php @@ -1528,7 +1528,18 @@ interface IDatabase { * * @param callable $callback * @param string $fname Caller name + * @since 1.32 + */ + public function onTransactionCommitOrIdle( callable $callback, $fname = __METHOD__ ); + + /** + * Alias for onTransactionCommitOrIdle() for backwards-compatibility + * + * @param callable $callback + * @param string $fname + * @return mixed * @since 1.20 + * @deprecated Since 1.32 */ public function onTransactionIdle( callable $callback, $fname = __METHOD__ ); @@ -1567,7 +1578,7 @@ interface IDatabase { * * Registering a callback here will not affect writesOrCallbacks() pending. * - * Since callbacks from this method or onTransactionIdle() can start and end transactions, + * Since callbacks from this or onTransactionCommitOrIdle() can start and end transactions, * a single call to IDatabase::commit might trigger multiple runs of the listener callbacks. * * @param string $name Callback name diff --git a/includes/profiler/output/ProfilerOutputDb.php b/includes/profiler/output/ProfilerOutputDb.php index 1c3d479d05..6e0085d8fc 100644 --- a/includes/profiler/output/ProfilerOutputDb.php +++ b/includes/profiler/output/ProfilerOutputDb.php @@ -56,7 +56,7 @@ class ProfilerOutputDb extends ProfilerOutput { } $fname = __METHOD__; - $dbw->onTransactionIdle( function ( Database $dbw ) use ( $stats, $fname ) { + $dbw->onTransactionCommitOrIdle( 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/db/LoadBalancerTest.php b/tests/phpunit/includes/db/LoadBalancerTest.php index 7462f1d530..cf605c19bd 100644 --- a/tests/phpunit/includes/db/LoadBalancerTest.php +++ b/tests/phpunit/includes/db/LoadBalancerTest.php @@ -365,13 +365,13 @@ class LoadBalancerTest extends MediaWikiTestCase { $tlCalls = 0; $lb->beginMasterChanges( __METHOD__ ); $ac = array_fill_keys( [ 'a', 'b', 'c', 'd' ], 0 ); - $conn1->onTransactionIdle( function () use ( &$ac, $conn1, $conn2 ) { + $conn1->onTransactionCommitOrIdle( function () use ( &$ac, $conn1, $conn2 ) { $ac['a'] = 1; - $conn2->onTransactionIdle( function () use ( &$ac, $conn1, $conn2 ) { + $conn2->onTransactionCommitOrIdle( function () use ( &$ac, $conn1, $conn2 ) { $ac['b'] = 1; - $conn1->onTransactionIdle( function () use ( &$ac, $conn1, $conn2 ) { + $conn1->onTransactionCommitOrIdle( function () use ( &$ac, $conn1, $conn2 ) { $ac['c'] = 1; - $conn1->onTransactionIdle( function () use ( &$ac, $conn1, $conn2 ) { + $conn1->onTransactionCommitOrIdle( function () use ( &$ac, $conn1, $conn2 ) { $ac['d'] = 1; } ); } ); diff --git a/tests/phpunit/includes/deferred/DeferredUpdatesTest.php b/tests/phpunit/includes/deferred/DeferredUpdatesTest.php index a1e41d97e8..3662c26646 100644 --- a/tests/phpunit/includes/deferred/DeferredUpdatesTest.php +++ b/tests/phpunit/includes/deferred/DeferredUpdatesTest.php @@ -358,7 +358,7 @@ class DeferredUpdatesTest extends MediaWikiTestCase { $this->assertEquals( [], $calls ); $dbw = wfGetDB( DB_MASTER ); - $dbw->onTransactionIdle( function () use ( &$calls, $callback2 ) { + $dbw->onTransactionCommitOrIdle( function () use ( &$calls, $callback2 ) { DeferredUpdates::addCallableUpdate( $callback2 ); $this->assertEquals( [], $calls ); $calls[] = 'oti'; diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php index 4596c764fc..b434396173 100644 --- a/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php +++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php @@ -1453,7 +1453,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase { $this->assertLastSql( 'BEGIN; ROLLBACK' ); $this->database->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE ); - $this->database->onTransactionIdle( $callback1, __METHOD__ ); + $this->database->onTransactionCommitOrIdle( $callback1, __METHOD__ ); $this->database->cancelAtomic( __METHOD__ ); $this->assertLastSql( 'BEGIN; ROLLBACK' ); @@ -1479,11 +1479,11 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase { ] ) ); $this->database->startAtomic( __METHOD__ . '_outer' ); - $this->database->onTransactionIdle( $callback1, __METHOD__ ); + $this->database->onTransactionCommitOrIdle( $callback1, __METHOD__ ); $this->database->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE ); - $this->database->onTransactionIdle( $callback2, __METHOD__ ); + $this->database->onTransactionCommitOrIdle( $callback2, __METHOD__ ); $this->database->cancelAtomic( __METHOD__ ); - $this->database->onTransactionIdle( $callback3, __METHOD__ ); + $this->database->onTransactionCommitOrIdle( $callback3, __METHOD__ ); $this->database->endAtomic( __METHOD__ . '_outer' ); $this->assertLastSql( implode( "; ", [ 'BEGIN', @@ -1640,7 +1640,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase { $this->database->startAtomic( __METHOD__ . '_outer' ); $this->database->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE ); $this->database->startAtomic( __METHOD__ . '_inner' ); - $this->database->onTransactionIdle( $callback1, __METHOD__ ); + $this->database->onTransactionCommitOrIdle( $callback1, __METHOD__ ); $this->database->onTransactionPreCommitOrIdle( $callback2, __METHOD__ ); $this->database->onTransactionResolution( $callback3, __METHOD__ ); $this->database->endAtomic( __METHOD__ . '_inner' ); @@ -1658,7 +1658,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase { $this->database->startAtomic( __METHOD__ . '_outer' ); $this->database->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE ); $this->database->startAtomic( __METHOD__ . '_inner', IDatabase::ATOMIC_CANCELABLE ); - $this->database->onTransactionIdle( $callback1, __METHOD__ ); + $this->database->onTransactionCommitOrIdle( $callback1, __METHOD__ ); $this->database->onTransactionPreCommitOrIdle( $callback2, __METHOD__ ); $this->database->onTransactionResolution( $callback3, __METHOD__ ); $this->database->endAtomic( __METHOD__ . '_inner' ); @@ -1676,7 +1676,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase { $this->database->startAtomic( __METHOD__ . '_outer' ); $atomicId = $this->database->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE ); $this->database->startAtomic( __METHOD__ . '_inner' ); - $this->database->onTransactionIdle( $callback1, __METHOD__ ); + $this->database->onTransactionCommitOrIdle( $callback1, __METHOD__ ); $this->database->onTransactionPreCommitOrIdle( $callback2, __METHOD__ ); $this->database->onTransactionResolution( $callback3, __METHOD__ ); $this->database->cancelAtomic( __METHOD__, $atomicId ); @@ -1691,7 +1691,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase { $this->database->startAtomic( __METHOD__ . '_outer' ); $atomicId = $this->database->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE ); $this->database->startAtomic( __METHOD__ . '_inner' ); - $this->database->onTransactionIdle( $callback1, __METHOD__ ); + $this->database->onTransactionCommitOrIdle( $callback1, __METHOD__ ); $this->database->onTransactionPreCommitOrIdle( $callback2, __METHOD__ ); $this->database->onTransactionResolution( $callback3, __METHOD__ ); try { @@ -1712,7 +1712,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase { $this->database->startAtomic( __METHOD__ . '_outer' ); $this->database->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE ); $this->database->startAtomic( __METHOD__ . '_inner' ); - $this->database->onTransactionIdle( $callback1, __METHOD__ ); + $this->database->onTransactionCommitOrIdle( $callback1, __METHOD__ ); $this->database->onTransactionPreCommitOrIdle( $callback2, __METHOD__ ); $this->database->onTransactionResolution( $callback3, __METHOD__ ); $this->database->cancelAtomic( __METHOD__ . '_inner' ); @@ -1729,7 +1729,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase { $this->database->startAtomic( __METHOD__ . '_outer' ); $this->database->startAtomic( __METHOD__, IDatabase::ATOMIC_CANCELABLE ); $this->database->startAtomic( __METHOD__ . '_inner' ); - $this->database->onTransactionIdle( $callback1, __METHOD__ ); + $this->database->onTransactionCommitOrIdle( $callback1, __METHOD__ ); $this->database->onTransactionPreCommitOrIdle( $callback2, __METHOD__ ); $this->database->onTransactionResolution( $callback3, __METHOD__ ); $wrapper->trxStatus = Database::STATUS_TRX_ERROR; @@ -1986,7 +1986,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase { public function testPrematureClose1() { $fname = __METHOD__; $this->database->begin( __METHOD__ ); - $this->database->onTransactionIdle( function () use ( $fname ) { + $this->database->onTransactionCommitOrIdle( function () use ( $fname ) { $this->database->query( 'SELECT 1', $fname ); } ); $this->database->delete( 'x', [ 'field' => 3 ], __METHOD__ ); @@ -2004,7 +2004,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase { try { $fname = __METHOD__; $this->database->startAtomic( __METHOD__ ); - $this->database->onTransactionIdle( function () use ( $fname ) { + $this->database->onTransactionCommitOrIdle( function () use ( $fname ) { $this->database->query( 'SELECT 1', $fname ); } ); $this->database->delete( 'x', [ 'field' => 3 ], __METHOD__ ); diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php index f08b3767dd..3f0dae633f 100644 --- a/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php +++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php @@ -172,7 +172,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase { } /** - * @covers Wikimedia\Rdbms\Database::onTransactionIdle + * @covers Wikimedia\Rdbms\Database::onTransactionCommitOrIdle * @covers Wikimedia\Rdbms\Database::runOnTransactionIdleCallbacks */ public function testTransactionIdle() { @@ -186,7 +186,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase { $flagSet = $db->getFlag( DBO_TRX ); }; - $db->onTransactionIdle( $callback, __METHOD__ ); + $db->onTransactionCommitOrIdle( $callback, __METHOD__ ); $this->assertTrue( $called, 'Callback reached' ); $this->assertFalse( $flagSet, 'DBO_TRX off in callback' ); $this->assertFalse( $db->getFlag( DBO_TRX ), 'DBO_TRX still default' ); @@ -194,7 +194,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase { $flagSet = null; $called = false; $db->startAtomic( __METHOD__ ); - $db->onTransactionIdle( $callback, __METHOD__ ); + $db->onTransactionCommitOrIdle( $callback, __METHOD__ ); $this->assertFalse( $called, 'Callback not reached during TRX' ); $db->endAtomic( __METHOD__ ); @@ -203,7 +203,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase { $this->assertFalse( $db->getFlag( DBO_TRX ), 'DBO_TRX restored to default' ); $db->clearFlag( DBO_TRX ); - $db->onTransactionIdle( + $db->onTransactionCommitOrIdle( function ( $trigger, IDatabase $db ) { $db->setFlag( DBO_TRX ); }, @@ -213,7 +213,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase { } /** - * @covers Wikimedia\Rdbms\Database::onTransactionIdle + * @covers Wikimedia\Rdbms\Database::onTransactionCommitOrIdle * @covers Wikimedia\Rdbms\Database::runOnTransactionIdleCallbacks */ public function testTransactionIdle_TRX() { @@ -237,14 +237,14 @@ class DatabaseTest extends PHPUnit\Framework\TestCase { $flagSet = $db->getFlag( DBO_TRX ); }; - $db->onTransactionIdle( $callback, __METHOD__ ); + $db->onTransactionCommitOrIdle( $callback, __METHOD__ ); $this->assertTrue( $called, 'Called when idle if DBO_TRX is set' ); $this->assertFalse( $flagSet, 'DBO_TRX off in callback' ); $this->assertTrue( $db->getFlag( DBO_TRX ), 'DBO_TRX still default' ); $called = false; $lbFactory->beginMasterChanges( __METHOD__ ); - $db->onTransactionIdle( $callback, __METHOD__ ); + $db->onTransactionCommitOrIdle( $callback, __METHOD__ ); $this->assertFalse( $called, 'Not called when lb-transaction is active' ); $lbFactory->commitMasterChanges( __METHOD__ ); @@ -252,7 +252,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase { $called = false; $lbFactory->beginMasterChanges( __METHOD__ ); - $db->onTransactionIdle( $callback, __METHOD__ ); + $db->onTransactionCommitOrIdle( $callback, __METHOD__ ); $this->assertFalse( $called, 'Not called when lb-transaction is active' ); $lbFactory->rollbackMasterChanges( __METHOD__ ); -- 2.20.1