Merge "Get rid of ApiTestCase::doLogin"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Wed, 11 Apr 2018 17:47:29 +0000 (17:47 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Wed, 11 Apr 2018 17:47:29 +0000 (17:47 +0000)
includes/libs/rdbms/database/Database.php
includes/libs/rdbms/database/IDatabase.php
includes/site/MediaWikiPageNameNormalizer.php
includes/site/MediaWikiSite.php
includes/site/Site.php
maintenance/resources/update-ooui.sh
tests/phpunit/includes/libs/rdbms/database/DatabaseSQLTest.php

index 5b259bd..8d38fb0 100644 (file)
@@ -898,34 +898,48 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                if ( $this->conn ) {
                        // Resolve any dangling transaction first
                        if ( $this->trxLevel ) {
-                               // Meaningful transactions should ideally have been resolved by now
-                               if ( $this->writesOrCallbacksPending() ) {
-                                       $this->queryLogger->warning(
-                                               __METHOD__ . ": writes or callbacks still pending.",
-                                               [ 'trace' => ( new RuntimeException() )->getTraceAsString() ]
-                                       );
+                               if ( $this->trxAtomicLevels ) {
                                        // Cannot let incomplete atomic sections be committed
-                                       if ( $this->trxAtomicLevels ) {
-                                               $levels = $this->flatAtomicSectionList();
-                                               $exception = new DBUnexpectedError(
-                                                       $this,
-                                                       __METHOD__ . ": atomic sections $levels are still open."
-                                               );
-                                       // Check if it is possible to properly commit and trigger callbacks
-                                       } elseif ( $this->trxEndCallbacksSuppressed ) {
+                                       $levels = $this->flatAtomicSectionList();
+                                       $exception = new DBUnexpectedError(
+                                               $this,
+                                               __METHOD__ . ": atomic sections $levels are still open."
+                                       );
+                               } elseif ( $this->trxAutomatic ) {
+                                       // Only the connection manager can commit non-empty DBO_TRX transactions
+                                       if ( $this->writesOrCallbacksPending() ) {
                                                $exception = new DBUnexpectedError(
                                                        $this,
-                                                       __METHOD__ . ': callbacks are suppressed; cannot properly commit.'
+                                                       __METHOD__ .
+                                                       ": mass commit/rollback of peer transaction required (DBO_TRX set)."
                                                );
                                        }
+                               } elseif ( $this->trxLevel ) {
+                                       // Commit explicit transactions as if this was commit()
+                                       $this->queryLogger->warning(
+                                               __METHOD__ . ": writes or callbacks still pending.",
+                                               [ 'trace' => ( new RuntimeException() )->getTraceAsString() ]
+                                       );
+                               }
+
+                               if ( $this->trxEndCallbacksSuppressed ) {
+                                       $exception = $exception ?: new DBUnexpectedError(
+                                               $this,
+                                               __METHOD__ . ': callbacks are suppressed; cannot properly commit.'
+                                       );
                                }
+
                                // Commit or rollback the changes and run any callbacks as needed
                                if ( $this->trxStatus === self::STATUS_TRX_OK && !$exception ) {
-                                       $this->commit( __METHOD__, self::TRANSACTION_INTERNAL );
+                                       $this->commit(
+                                               __METHOD__,
+                                               $this->trxAutomatic ? self::FLUSHING_INTERNAL : self::FLUSHING_ONE
+                                       );
                                } else {
-                                       $this->rollback( __METHOD__, self::TRANSACTION_INTERNAL );
+                                       $this->rollback( __METHOD__, self::FLUSHING_INTERNAL );
                                }
                        }
+
                        // Close the actual connection in the binding handle
                        $closed = $this->closeConnection();
                        $this->conn = false;
@@ -3513,6 +3527,11 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        final public function begin( $fname = __METHOD__, $mode = self::TRANSACTION_EXPLICIT ) {
+               static $modes = [ self::TRANSACTION_EXPLICIT, self::TRANSACTION_INTERNAL ];
+               if ( !in_array( $mode, $modes, true ) ) {
+                       throw new DBUnexpectedError( $this, "$fname: invalid mode parameter '$mode'." );
+               }
+
                // Protect against mismatched atomic section, transaction nesting, and snapshot loss
                if ( $this->trxLevel ) {
                        if ( $this->trxAtomicLevels ) {
@@ -3571,9 +3590,14 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                $this->trxLevel = 1;
        }
 
-       final public function commit( $fname = __METHOD__, $flush = '' ) {
+       final public function commit( $fname = __METHOD__, $flush = self::FLUSHING_ONE ) {
+               static $modes = [ self::FLUSHING_ONE, self::FLUSHING_ALL_PEERS, self::FLUSHING_INTERNAL ];
+               if ( !in_array( $flush, $modes, true ) ) {
+                       throw new DBUnexpectedError( $this, "$fname: invalid flush parameter '$flush'." );
+               }
+
                if ( $this->trxLevel && $this->trxAtomicLevels ) {
-                       // There are still atomic sections open. This cannot be ignored
+                       // There are still atomic sections open; this cannot be ignored
                        $levels = $this->flatAtomicSectionList();
                        throw new DBUnexpectedError(
                                $this,
index 5876d6b..ff561f1 100644 (file)
@@ -54,9 +54,11 @@ interface IDatabase {
        /** @var string Atomic section is cancelable */
        const ATOMIC_CANCELABLE = 'cancelable';
 
-       /** @var string Transaction operation comes from service managing all DBs */
+       /** @var string Commit/rollback is from outside the IDatabase handle and connection manager */
+       const FLUSHING_ONE = '';
+       /** @var string Commit/rollback is from the connection manager for the IDatabase handle */
        const FLUSHING_ALL_PEERS = 'flush';
-       /** @var string Transaction operation comes from the database class internally */
+       /** @var string Commit/rollback is from the IDatabase handle internally */
        const FLUSHING_INTERNAL = 'flush';
 
        /** @var string Do not remember the prior flags */
index 3e073f0..8a12c4f 100644 (file)
@@ -54,6 +54,7 @@ class MediaWikiPageNameNormalizer {
         * Returns the normalized form of the given page title, using the
         * normalization rules of the given site. If the given title is a redirect,
         * the redirect will be resolved and the redirect target is returned.
+        * Only titles of existing pages will be returned.
         *
         * @note This actually makes an API request to the remote site, so beware
         *   that this function is slow and depends on an external service.
@@ -65,7 +66,9 @@ class MediaWikiPageNameNormalizer {
         * @param string $pageName
         * @param string $apiUrl
         *
-        * @return string|false
+        * @return string|false The normalized form of the title,
+        * or false to indicate an invalid title, a missing page,
+        * or some other kind of error.
         * @throws \MWException
         */
        public function normalizePageName( $pageName, $apiUrl ) {
index 0ff7e8b..e1e7ce6 100644 (file)
@@ -65,6 +65,7 @@ class MediaWikiSite extends Site {
         * Returns the normalized form of the given page title, using the
         * normalization rules of the given site. If the given title is a redirect,
         * the redirect will be resolved and the redirect target is returned.
+        * Only titles of existing pages will be returned.
         *
         * @note This actually makes an API request to the remote site, so beware
         *   that this function is slow and depends on an external service.
@@ -79,7 +80,9 @@ class MediaWikiSite extends Site {
         *
         * @param string $pageName
         *
-        * @return string|false
+        * @return string|false The normalized form of the title,
+        * or false to indicate an invalid title, a missing page,
+        * or some other kind of error.
         * @throws MWException
         */
        public function normalizePageName( $pageName ) {
index 55aad77..f5e3f22 100644 (file)
@@ -382,8 +382,10 @@ class Site implements Serializable {
        }
 
        /**
-        * Returns $pageName without changes.
-        * Subclasses may override this to apply some kind of normalization.
+        * Attempt to normalize the page name in some fashion.
+        * May return false to indicate various kinds of failure.
+        *
+        * This implementation returns $pageName without changes.
         *
         * @see Site::normalizePageName
         *
index 673d507..fc8a0e9 100755 (executable)
@@ -11,7 +11,7 @@ fi
 
 REPO_DIR=$(cd "$(dirname $0)/../.."; pwd) # Root dir of the git repo working tree
 TARGET_DIR="resources/lib/oojs-ui" # Destination relative to the root of the repo
-NPM_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'update-oojs-ui') # e.g. /tmp/update-oojs-ui.rI0I5Vir
+NPM_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'update-ooui') # e.g. /tmp/update-ooui.rI0I5Vir
 
 # Prepare working tree
 cd "$REPO_DIR"
@@ -72,13 +72,6 @@ cp ./node_modules/oojs-ui/dist/themes/wikimediaui/images/textures/*.{gif,svg} "$
 cp ./node_modules/oojs-ui/src/themes/wikimediaui/*.json "$REPO_DIR/$TARGET_DIR/themes/wikimediaui"
 
 # Apex theme icons, indicators, and textures
-mkdir -p "$REPO_DIR/$TARGET_DIR/themes/apex/images/icons"
-cp ./node_modules/oojs-ui/dist/themes/apex/images/icons/*.{svg,png} "$REPO_DIR/$TARGET_DIR/themes/apex/images/icons"
-mkdir -p "$REPO_DIR/$TARGET_DIR/themes/apex/images/indicators"
-cp ./node_modules/oojs-ui/dist/themes/apex/images/indicators/*.{svg,png} "$REPO_DIR/$TARGET_DIR/themes/apex/images/indicators"
-mkdir -p "$REPO_DIR/$TARGET_DIR/themes/apex/images/textures"
-cp ./node_modules/oojs-ui/dist/themes/apex/images/textures/*.{gif,svg} "$REPO_DIR/$TARGET_DIR/themes/apex/images/textures"
-
 cp ./node_modules/oojs-ui/src/themes/apex/*.json "$REPO_DIR/$TARGET_DIR/themes/apex"
 
 # WikimediaUI LESS variables for sharing
index 665e953..2c07739 100644 (file)
@@ -1687,4 +1687,42 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
                $this->assertLastSql( 'BEGIN; DELETE FROM x WHERE field = \'3\'; ROLLBACK' );
                $this->assertEquals( 0, $this->database->trxLevel() );
        }
+
+       /**
+        * @covers \Wikimedia\Rdbms\Database::close
+        */
+       public function testPrematureClose3() {
+               try {
+                       $this->database->setFlag( IDatabase::DBO_TRX );
+                       $this->database->delete( 'x', [ 'field' => 3 ], __METHOD__ );
+                       $this->assertEquals( 1, $this->database->trxLevel() );
+                       $this->database->close();
+                       $this->fail( 'Expected exception not thrown' );
+               } catch ( DBUnexpectedError $ex ) {
+                       $this->assertSame(
+                               'Wikimedia\Rdbms\Database::close: ' .
+                               'mass commit/rollback of peer transaction required (DBO_TRX set).',
+                               $ex->getMessage()
+                       );
+               }
+
+               $this->assertFalse( $this->database->isOpen() );
+               $this->assertLastSql( 'BEGIN; DELETE FROM x WHERE field = \'3\'; ROLLBACK' );
+               $this->assertEquals( 0, $this->database->trxLevel() );
+       }
+
+       /**
+        * @covers \Wikimedia\Rdbms\Database::close
+        */
+       public function testPrematureClose4() {
+               $this->database->setFlag( IDatabase::DBO_TRX );
+               $this->database->query( 'SELECT 1', __METHOD__ );
+               $this->assertEquals( 1, $this->database->trxLevel() );
+               $this->database->close();
+               $this->database->clearFlag( IDatabase::DBO_TRX );
+
+               $this->assertFalse( $this->database->isOpen() );
+               $this->assertLastSql( 'BEGIN; SELECT 1; COMMIT' );
+               $this->assertEquals( 0, $this->database->trxLevel() );
+       }
 }