Remove unused custom transaction logic from DataUpdate

Change-Id: Ife65e4e90a35395e87f4f487f1cb871b67d92aa1

diff --git a/includes/deferred/DataUpdate.php b/includes/deferred/DataUpdate.php
index 281ac24..cad89b1 100644 (file)
--- 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 ca3500e..d0b12a0 100644 (file)
--- 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 6124a71..69f8d13 100644 (file)
--- 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 c7163ea..25e8841 100644 (file)
--- 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 3309352..9cc3ffd 100644 (file)
--- 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;