Merge "resourceloader: Move mw.loader qunit tests to a separate file"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 18 Aug 2016 17:42:53 +0000 (17:42 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 18 Aug 2016 17:42:53 +0000 (17:42 +0000)
includes/api/ApiQueryBacklinksprop.php
includes/db/Database.php
includes/db/IDatabase.php
languages/Language.php
resources/assets/licenses/README
tests/phpunit/includes/db/DatabaseTest.php
tests/phpunit/includes/libs/ObjectFactoryTest.php

index 3810e90..236fb9e 100644 (file)
@@ -53,6 +53,7 @@ class ApiQueryBacklinksprop extends ApiQueryGeneratorBase {
                        'code' => 'lh',
                        'prefix' => 'pl',
                        'linktable' => 'pagelinks',
+                       'indexes' => [ 'pl_namespace', 'pl_backlinks_namespace' ],
                        'from_namespace' => true,
                        'showredirects' => true,
                ],
@@ -60,6 +61,7 @@ class ApiQueryBacklinksprop extends ApiQueryGeneratorBase {
                        'code' => 'ti',
                        'prefix' => 'tl',
                        'linktable' => 'templatelinks',
+                       'indexes' => [ 'tl_namespace', 'tl_backlinks_namespace' ],
                        'from_namespace' => true,
                        'showredirects' => true,
                ],
@@ -67,6 +69,7 @@ class ApiQueryBacklinksprop extends ApiQueryGeneratorBase {
                        'code' => 'fu',
                        'prefix' => 'il',
                        'linktable' => 'imagelinks',
+                       'indexes' => [ 'il_to', 'il_backlinks_namespace' ],
                        'from_namespace' => true,
                        'to_namespace' => NS_FILE,
                        'exampletitle' => 'File:Example.jpg',
@@ -249,6 +252,18 @@ class ApiQueryBacklinksprop extends ApiQueryGeneratorBase {
                // Override any ORDER BY from above with what we calculated earlier.
                $this->addOption( 'ORDER BY', array_keys( $sortby ) );
 
+               // MySQL's optimizer chokes if we have too many values in "$bl_title IN
+               // (...)" and chooses the wrong index, so specify the correct index to
+               // use for the query. See T139056 for details.
+               if ( !empty( $settings['indexes'] ) ) {
+                       list( $idxNoFromNS, $idxWithFromNS ) = $settings['indexes'];
+                       if ( $params['namespace'] !== null && !empty( $settings['from_namespace'] ) ) {
+                               $this->addOption( 'USE INDEX', [ $settings['linktable'] => $idxWithFromNS ] );
+                       } else {
+                               $this->addOption( 'USE INDEX', [ $settings['linktable'] => $idxNoFromNS ] );
+                       }
+               }
+
                $this->addOption( 'LIMIT', $params['limit'] + 1 );
 
                $res = $this->select( __METHOD__ );
index d492def..be0399d 100644 (file)
@@ -2658,12 +2658,14 @@ abstract class DatabaseBase implements IDatabase {
        final public function doAtomicSection( $fname, callable $callback ) {
                $this->startAtomic( $fname );
                try {
-                       call_user_func_array( $callback, [ $this, $fname ] );
+                       $res = call_user_func_array( $callback, [ $this, $fname ] );
                } catch ( Exception $e ) {
                        $this->rollback( $fname );
                        throw $e;
                }
                $this->endAtomic( $fname );
+
+               return $res;
        }
 
        final public function begin( $fname = __METHOD__, $mode = self::TRANSACTION_EXPLICIT ) {
@@ -3289,8 +3291,16 @@ abstract class DatabaseBase implements IDatabase {
                }
 
                $unlocker = new ScopedCallback( function () use ( $lockKey, $fname ) {
-                       $this->commit( __METHOD__, self::FLUSHING_INTERNAL );
-                       $this->unlock( $lockKey, $fname );
+                       if ( $this->trxLevel() ) {
+                               // There is a good chance an exception was thrown, causing any early return
+                               // from the caller. Let any error handler get a chance to issue rollback().
+                               // If there isn't one, let the error bubble up and trigger server-side rollback.
+                               $this->onTransactionResolution( function () use ( $lockKey, $fname ) {
+                                       $this->unlock( $lockKey, $fname );
+                               } );
+                       } else {
+                               $this->unlock( $lockKey, $fname );
+                       }
                } );
 
                $this->commit( __METHOD__, self::FLUSHING_INTERNAL );
index fce5aa8..bdab09e 100644 (file)
@@ -1347,6 +1347,7 @@ interface IDatabase {
         *
         * @param string $fname Caller name (usually __METHOD__)
         * @param callable $callback Callback that issues DB updates
+        * @return mixed $res Result of the callback (since 1.28)
         * @throws DBError
         * @throws RuntimeException
         * @throws UnexpectedValueException
@@ -1358,6 +1359,10 @@ interface IDatabase {
         * Begin a transaction. If a transaction is already in progress,
         * that transaction will be committed before the new transaction is started.
         *
+        * Only call this from code with outer transcation scope.
+        * See https://www.mediawiki.org/wiki/Database_transactions for details.
+        * Nesting of transactions is not supported.
+        *
         * Note that when the DBO_TRX flag is set (which is usually the case for web
         * requests, but not for maintenance scripts), any previous database query
         * will have started a transaction automatically.
@@ -1376,6 +1381,8 @@ interface IDatabase {
         * Commits a transaction previously started using begin().
         * If no transaction is in progress, a warning is issued.
         *
+        * Only call this from code with outer transcation scope.
+        * See https://www.mediawiki.org/wiki/Database_transactions for details.
         * Nesting of transactions is not supported.
         *
         * @param string $fname
@@ -1396,7 +1403,11 @@ interface IDatabase {
         * Rollback a transaction previously started using begin().
         * If no transaction is in progress, a warning is issued.
         *
-        * No-op on non-transactional databases.
+        * Only call this from code with outer transcation scope.
+        * See https://www.mediawiki.org/wiki/Database_transactions for details.
+        * Nesting of transactions is not supported. If a serious unexpected error occurs,
+        * throwing an Exception is preferrable, using a pre-installed error handler to trigger
+        * rollback (in any case, failure to issue COMMIT will cause rollback server-side).
         *
         * @param string $fname
         * @param string $flush Flush flag, set to a situationally valid IDatabase::FLUSHING_*
@@ -1567,10 +1578,14 @@ interface IDatabase {
        /**
         * Acquire a named lock, flush any transaction, and return an RAII style unlocker object
         *
+        * Only call this from outer transcation scope and when only one DB will be affected.
+        * See https://www.mediawiki.org/wiki/Database_transactions for details.
+        *
         * This is suitiable for transactions that need to be serialized using cooperative locks,
         * where each transaction can see each others' changes. Any transaction is flushed to clear
         * out stale REPEATABLE-READ snapshot data. Once the returned object falls out of PHP scope,
-        * any transaction will be committed and the lock will be released.
+        * the lock will be released unless a transaction is active. If one is active, then the lock
+        * will be released when it either commits or rolls back.
         *
         * If the lock acquisition failed, then no transaction flush happens, and null is returned.
         *
index d96710a..8b39d77 100644 (file)
  * @defgroup Language Language
  */
 
-if ( !defined( 'MEDIAWIKI' ) ) {
-       echo "This file is part of MediaWiki, it is not a valid entry point.\n";
-       exit( 1 );
-}
-
 use CLDRPluralRuleParser\Evaluator;
 
 /**
index 0f5cf51..dae2549 100644 (file)
@@ -1,4 +1,4 @@
 These license icons are used in LocalSettings.php files that are generated by
-the installer. Although public domain has been removed from the installer as
-an option, the image needs to remain here to support installations which refer
-to it in LocalSettings.php.
+the installer. Although "Public domain" has been removed from the installer as
+an option, the public-domain.png image needs to remain here to support older
+installations that refer to it in LocalSettings.php.
index 723f1c3..7e55a73 100644 (file)
@@ -285,8 +285,42 @@ class DatabaseTest extends MediaWikiTestCase {
                        $called = true;
                        $db->setFlag( DBO_TRX );
                } );
-               $db->rollback( __METHOD__ );
+               $db->rollback( __METHOD__, IDatabase::FLUSHING_ALL_PEERS );
                $this->assertFalse( $db->getFlag( DBO_TRX ), 'DBO_TRX restored to default' );
                $this->assertTrue( $called, 'Callback reached' );
        }
+
+       public function testGetScopedLock() {
+               $db = $this->db;
+
+               $db->setFlag( DBO_TRX );
+               try {
+                       $this->badLockingMethodImplicit( $db );
+               } catch ( RunTimeException $e ) {
+                       $this->assertTrue( $db->trxLevel() > 0, "Transaction not committed." );
+               }
+               $db->clearFlag( DBO_TRX );
+               $db->rollback( __METHOD__, IDatabase::FLUSHING_ALL_PEERS );
+               $this->assertTrue( $db->lockIsFree( 'meow', __METHOD__ ) );
+
+               try {
+                       $this->badLockingMethodExplicit( $db );
+               } catch ( RunTimeException $e ) {
+                       $this->assertTrue( $db->trxLevel() > 0, "Transaction not committed." );
+               }
+               $db->rollback( __METHOD__, IDatabase::FLUSHING_ALL_PEERS );
+               $this->assertTrue( $db->lockIsFree( 'meow', __METHOD__ ) );
+       }
+
+       private function badLockingMethodImplicit( IDatabase $db ) {
+               $lock = $db->getScopedLockAndFlush( 'meow', __METHOD__, 1 );
+               $db->query( "SELECT 1" ); // trigger DBO_TRX
+               throw new RunTimeException( "Uh oh!" );
+       }
+
+       private function badLockingMethodExplicit( IDatabase $db ) {
+               $lock = $db->getScopedLockAndFlush( 'meow', __METHOD__, 1 );
+               $db->begin( __METHOD__ );
+               throw new RunTimeException( "Uh oh!" );
+       }
 }
index a4871a6..f8dda6f 100644 (file)
@@ -44,6 +44,7 @@ class ObjectFactoryTest extends PHPUnit_Framework_TestCase {
 
        /**
         * @covers ObjectFactory::getObjectFromSpec
+        * @covers ObjectFactory::expandClosures
         */
        public function testClosureExpansionEnabled() {
                $obj = ObjectFactory::getObjectFromSpec( [
@@ -80,6 +81,44 @@ class ObjectFactoryTest extends PHPUnit_Framework_TestCase {
                $this->assertSame( 'unwrapped', $obj->setterArgs[0] );
        }
 
+       /**
+        * @covers ObjectFactory::getObjectFromSpec
+        */
+       public function testGetObjectFromFactory() {
+               $args = [ 'a', 'b' ];
+               $obj = ObjectFactory::getObjectFromSpec( [
+                       'factory' => function ( $a, $b ) {
+                               return new ObjectFactoryTestFixture( $a, $b );
+                       },
+                       'args' => $args,
+               ] );
+               $this->assertSame( $args, $obj->args );
+       }
+
+       /**
+        * @covers ObjectFactory::getObjectFromSpec
+        * @expectedException InvalidArgumentException
+        */
+       public function testGetObjectFromInvalid() {
+               $args = [ 'a', 'b' ];
+               $obj = ObjectFactory::getObjectFromSpec( [
+                       // Missing 'class' or 'factory'
+                       'args' => $args,
+               ] );
+       }
+
+       /**
+        * @covers ObjectFactory::getObjectFromSpec
+        * @dataProvider provideConstructClassInstance
+        */
+       public function testGetObjectFromClass( $args ) {
+               $obj = ObjectFactory::getObjectFromSpec( [
+                       'class' => 'ObjectFactoryTestFixture',
+                       'args' => $args,
+               ] );
+               $this->assertSame( $args, $obj->args );
+       }
+
        /**
         * @covers ObjectFactory::constructClassInstance
         * @dataProvider provideConstructClassInstance
@@ -91,7 +130,7 @@ class ObjectFactoryTest extends PHPUnit_Framework_TestCase {
                $this->assertSame( $args, $obj->args );
        }
 
-       public function provideConstructClassInstance() {
+       public static function provideConstructClassInstance() {
                // These args go to 11. I thought about making 10 one louder, but 11!
                return [
                        '0 args' => [ [] ],
@@ -110,6 +149,7 @@ class ObjectFactoryTest extends PHPUnit_Framework_TestCase {
        }
 
        /**
+        * @covers ObjectFactory::constructClassInstance
         * @expectedException InvalidArgumentException
         */
        public function testNamedArgs() {