From: Aaron Schulz Date: Thu, 1 Sep 2016 00:03:39 +0000 (-0700) Subject: Remove unused custom transaction logic from DataUpdate X-Git-Tag: 1.31.0-rc.0~5798 X-Git-Url: http://git.cyclocoop.org/%24self?a=commitdiff_plain;h=ef1c17b77d7f8a47d3db098db3acd3f162bbdaa8;p=lhc%2Fweb%2Fwiklou.git Remove unused custom transaction logic from DataUpdate Change-Id: Ife65e4e90a35395e87f4f487f1cb871b67d92aa1 --- diff --git a/includes/deferred/DataUpdate.php b/includes/deferred/DataUpdate.php index 281ac24d9c..cad89b1479 100644 --- a/includes/deferred/DataUpdate.php +++ b/includes/deferred/DataUpdate.php @@ -24,10 +24,6 @@ /** * Abstract base class for update jobs that do something with some secondary * data extracted from article. - * - * @note subclasses should NOT start or commit transactions in their doUpdate() method, - * a transaction will automatically be wrapped around the update. If need be, - * subclasses can override the beginTransaction() and commitTransaction() methods. */ abstract class DataUpdate implements DeferrableUpdate { /** @var mixed Result from LBFactory::getEmptyTransactionTicket() */ @@ -45,30 +41,6 @@ abstract class DataUpdate implements DeferrableUpdate { $this->ticket = $ticket; } - /** - * Begin an appropriate transaction, if any. - * This default implementation does nothing. - */ - public function beginTransaction() { - // noop - } - - /** - * Commit the transaction started via beginTransaction, if any. - * This default implementation does nothing. - */ - public function commitTransaction() { - // noop - } - - /** - * Abort / roll back the transaction started via beginTransaction, if any. - * This default implementation does nothing. - */ - public function rollbackTransaction() { - // noop - } - /** * Convenience method, calls doUpdate() on every DataUpdate in the array. * @@ -91,44 +63,8 @@ abstract class DataUpdate implements DeferrableUpdate { $updates = self::enqueueUpdates( $updates ); } - if ( !count( $updates ) ) { - return; // nothing to do - } - - $open_transactions = []; - $exception = null; - - try { - // begin transactions - foreach ( $updates as $update ) { - $update->beginTransaction(); - $open_transactions[] = $update; - } - - // do work - foreach ( $updates as $update ) { - $update->doUpdate(); - } - - // commit transactions - while ( count( $open_transactions ) > 0 ) { - $trans = array_pop( $open_transactions ); - $trans->commitTransaction(); - } - } catch ( Exception $ex ) { - $exception = $ex; - wfDebug( "Caught exception, will rethrow after rollback: " . - $ex->getMessage() . "\n" ); - } - - // rollback remaining transactions - while ( count( $open_transactions ) > 0 ) { - $trans = array_pop( $open_transactions ); - $trans->rollbackTransaction(); - } - - if ( $exception ) { - throw $exception; // rethrow after cleanup + foreach ( $updates as $update ) { + $update->doUpdate(); } } diff --git a/includes/deferred/LinksDeletionUpdate.php b/includes/deferred/LinksDeletionUpdate.php index ca3500eae2..d0b12a0416 100644 --- a/includes/deferred/LinksDeletionUpdate.php +++ b/includes/deferred/LinksDeletionUpdate.php @@ -42,6 +42,8 @@ class LinksDeletionUpdate extends DataUpdate implements EnqueueableDataUpdate { * @throws MWException */ function __construct( WikiPage $page, $pageId = null, $timestamp = null ) { + parent::__construct(); + $this->page = $page; if ( $pageId ) { $this->pageId = $pageId; // page ID at time of deletion diff --git a/includes/deferred/LinksUpdate.php b/includes/deferred/LinksUpdate.php index 6124a71eac..69f8d133bf 100644 --- a/includes/deferred/LinksUpdate.php +++ b/includes/deferred/LinksUpdate.php @@ -108,6 +108,8 @@ class LinksUpdate extends DataUpdate implements EnqueueableDataUpdate { * @throws MWException */ function __construct( Title $title, ParserOutput $parserOutput, $recursive = true ) { + parent::__construct(); + $this->mTitle = $title; $this->mId = $title->getArticleID( Title::GAID_FOR_UPDATE ); diff --git a/includes/deferred/SqlDataUpdate.php b/includes/deferred/SqlDataUpdate.php index c7163ea1b5..25e884114b 100644 --- a/includes/deferred/SqlDataUpdate.php +++ b/includes/deferred/SqlDataUpdate.php @@ -22,82 +22,17 @@ */ /** - * Abstract base class for update jobs that put some secondary data extracted - * from article content into the database. - * - * @note subclasses should NOT start or commit transactions in their doUpdate() method, - * a transaction will automatically be wrapped around the update. Starting another - * one would break the outer transaction bracket. If need be, subclasses can override - * the beginTransaction() and commitTransaction() methods. - * * @deprecated Since 1.28 Use DataUpdate directly, injecting the database */ abstract class SqlDataUpdate extends DataUpdate { /** @var IDatabase Database connection reference */ protected $mDb; - /** @var array SELECT options to be used (array) */ protected $mOptions = []; - /** @var bool Whether a transaction is open on this object (internal use only!) */ - private $mHasTransaction; - - /** @var bool Whether this update should be wrapped in a transaction */ - protected $mUseTransaction; - - /** - * Constructor - * - * @param bool $withTransaction Whether this update should be wrapped in a - * transaction (default: true). A transaction is only started if no - * transaction is already in progress, see beginTransaction() for details. - */ - public function __construct( $withTransaction = true ) { + public function __construct() { parent::__construct(); $this->mDb = wfGetLB()->getLazyConnectionRef( DB_MASTER ); - - $this->mWithTransaction = $withTransaction; - $this->mHasTransaction = false; - } - - /** - * Begin a database transaction, if $withTransaction was given as true in - * the constructor for this SqlDataUpdate. - * - * Because nested transactions are not supported by the Database class, - * this implementation checks Database::trxLevel() and only opens a - * transaction if none is already active. - */ - public function beginTransaction() { - if ( !$this->mWithTransaction ) { - return; - } - - // NOTE: nested transactions are not supported, only start a transaction if none is open - if ( $this->mDb->trxLevel() === 0 ) { - $this->mDb->begin( get_class( $this ) . '::beginTransaction' ); - $this->mHasTransaction = true; - } - } - - /** - * Commit the database transaction started via beginTransaction (if any). - */ - public function commitTransaction() { - if ( $this->mHasTransaction ) { - $this->mDb->commit( get_class( $this ) . '::commitTransaction' ); - $this->mHasTransaction = false; - } - } - - /** - * Abort the database transaction started via beginTransaction (if any). - */ - public function abortTransaction() { - if ( $this->mHasTransaction ) { // XXX: actually... maybe always? - $this->mDb->rollback( get_class( $this ) . '::abortTransaction' ); - $this->mHasTransaction = false; - } } } diff --git a/tests/phpunit/includes/deferred/LinksUpdateTest.php b/tests/phpunit/includes/deferred/LinksUpdateTest.php index 3309352fde..9cc3ffdab7 100644 --- a/tests/phpunit/includes/deferred/LinksUpdateTest.php +++ b/tests/phpunit/includes/deferred/LinksUpdateTest.php @@ -369,10 +369,7 @@ class LinksUpdateTest extends MediaWikiLangTestCase { ) { $update = new LinksUpdate( $title, $parserOutput ); - // NOTE: make sure LinksUpdate does not generate warnings when called inside a transaction. - $update->beginTransaction(); $update->doUpdate(); - $update->commitTransaction(); $this->assertSelect( $table, $fields, $condition, $expectedRows ); return $update;