Merge "Fix use of GenderCache in ApiPageSet::processTitlesArray"

diff --git a/includes/api/ApiPageSet.php b/includes/api/ApiPageSet.php
index 6afb018..e68676a 100644 (file)
--- a/includes/api/ApiPageSet.php
+++ b/includes/api/ApiPageSet.php
@@ -1166,7 +1166,8 @@ class ApiPageSet extends ApiBase {
                $services = MediaWikiServices::getInstance();
                $contLang = $services->getContentLanguage();
 
-               foreach ( $titles as $title ) {
+               $titleObjects = [];
+               foreach ( $titles as $index => $title ) {
                        if ( is_string( $title ) ) {
                                try {
                                        $titleObj = Title::newFromTextThrow( $title, $this->mDefaultNamespace );
@@ -1185,6 +1186,16 @@ class ApiPageSet extends ApiBase {
                        } else {
                                $titleObj = $title;
                        }
+
+                       $titleObjects[$index] = $titleObj;
+               }
+
+               // Get gender information
+               $genderCache = $services->getGenderCache();
+               $genderCache->doTitlesArray( $titleObjects, __METHOD__ );
+
+               foreach ( $titleObjects as $index => $titleObj ) {
+                       $title = is_string( $titles[$index] ) ? $titles[$index] : false;
                        $unconvertedTitle = $titleObj->getPrefixedText();
                        $titleWasConverted = false;
                        if ( $titleObj->isExternal() ) {
@@ -1197,7 +1208,7 @@ class ApiPageSet extends ApiBase {
                                ) {
                                        // Language::findVariantLink will modify titleText and titleObj into
                                        // the canonical variant if possible
-                                       $titleText = is_string( $title ) ? $title : $titleObj->getPrefixedText();
+                                       $titleText = $title !== false ? $title : $titleObj->getPrefixedText();
                                        $contLang->findVariantLink( $titleText, $titleObj );
                                        $titleWasConverted = $unconvertedTitle !== $titleObj->getPrefixedText();
                                }
@@ -1245,24 +1256,13 @@ class ApiPageSet extends ApiBase {
                        if ( $titleWasConverted ) {
                                $this->mConvertedTitles[$unconvertedTitle] = $titleObj->getPrefixedText();
                                // In this case the page can't be Special.
-                               if ( is_string( $title ) && $title !== $unconvertedTitle ) {
+                               if ( $title !== false && $title !== $unconvertedTitle ) {
                                        $this->mNormalizedTitles[$title] = $unconvertedTitle;
                                }
-                       } elseif ( is_string( $title ) && $title !== $titleObj->getPrefixedText() ) {
+                       } elseif ( $title !== false && $title !== $titleObj->getPrefixedText() ) {
                                $this->mNormalizedTitles[$title] = $titleObj->getPrefixedText();
                        }
-
-                       // Need gender information
-                       if (
-                               $services->getNamespaceInfo()->
-                                       hasGenderDistinction( $titleObj->getNamespace() )
-                       ) {
-                               $usernames[] = $titleObj->getText();
-                       }
                }
-               // Get gender information
-               $genderCache = $services->getGenderCache();
-               $genderCache->doQuery( $usernames, __METHOD__ );
 
                return $linkBatch;
        }

diff --git a/tests/phpunit/includes/api/ApiPageSetTest.php b/tests/phpunit/includes/api/ApiPageSetTest.php
index b9e4645..fdc9c1b 100644 (file)
--- a/tests/phpunit/includes/api/ApiPageSetTest.php
+++ b/tests/phpunit/includes/api/ApiPageSetTest.php
@@ -1,5 +1,8 @@
 <?php
 
+use MediaWiki\MediaWikiServices;
+use Wikimedia\TestingAccessWrapper;
+
 /**
  * @group API
  * @group medium
@@ -176,4 +179,44 @@ class ApiPageSetTest extends ApiTestCase {
                        3 => [ "$userDbkey/subpage" => -3 ],
                ], $pageSet->getAllTitlesByNamespace() );
        }
+
+       /**
+        * Test that ApiPageSet is calling GenderCache for provided user names to prefill the
+        * GenderCache and avoid a performance issue when loading each users' gender on it's own.
+        * The test is setting the "missLimit" to 0 on the GenderCache to trigger misses logic.
+        * When the "misses" property is no longer 0 at the end of the test,
+        * something was requested which is not part of the cache. Than the test is failing.
+        */
+       public function testGenderCaching() {
+               // Set up the user namespace to have gender aliases to trigger the gender cache
+               $this->setMwGlobals( [
+                       'wgExtraGenderNamespaces' => [ NS_USER => [ 'male' => 'Male', 'female' => 'Female' ] ]
+               ] );
+               $this->overrideMwServices();
+
+               // User names to test with - it is not needed that the user exists in the database
+               // to trigger gender cache
+               $userNames = [
+                       'Female',
+                       'Unknown',
+                       'Male',
+               ];
+
+               // Prepare the gender cache for testing - this is a fresh instance due to service override
+               $genderCache = TestingAccessWrapper::newFromObject(
+                       MediaWikiServices::getInstance()->getGenderCache()
+               );
+               $genderCache->missLimit = 0;
+
+               // Do an api request to trigger ApiPageSet code
+               $this->doApiRequest( [
+                       'action' => 'query',
+                       'titles' => 'User:' . implode( '|User:', $userNames ),
+               ] );
+
+               $this->assertEquals( 0, $genderCache->misses,
+                       'ApiPageSet does not prefill the gender cache correctly' );
+               $this->assertEquals( $userNames, array_keys( $genderCache->cache ),
+                       'ApiPageSet does not prefill all users into the gender cache' );
+       }
 }