resourceloader: Improve titleInfo docs and simplify title key

* Document the structure of the in-process $titleInfo cache.
  Specifically, specify that it is not the value from getTitleInfo(),
  but rather a container for zero or more versions of such values.
  The reason this is fragmented is because ResourceLoaderContext
  is a parameter to most methods and as such, makes everything
  variable. Tracked as T99107.

* Make various bits easier to understand by consistently refering
  to the container keys as "batchKey", and referring to the internal
  keys as "titleKey".

* Centralise title key logic by moving to private method.

* Replace the internal creation of titleKey to be based on LinkTarget
  with plain namespace IDs and db keys, instead of invoking the
  expensive getPrefixedTitle function which involves quite a lot
  of overhead (TitleCodec, GenderCache, Database, Language,
  LocalisationCache, ..).

Change-Id: I701e5156ef7815a0e36caefae5871524eff3f688

diff --git a/includes/resourceloader/ResourceLoaderWikiModule.php b/includes/resourceloader/ResourceLoaderWikiModule.php
index 5b512af..0926b60 100644 (file)
--- a/includes/resourceloader/ResourceLoaderWikiModule.php
+++ b/includes/resourceloader/ResourceLoaderWikiModule.php
@@ -22,6 +22,7 @@
  * @author Roan Kattouw
  */
 
+use MediaWiki\Linker\LinkTarget;
 use Wikimedia\Rdbms\Database;
 use Wikimedia\Rdbms\IDatabase;
 
@@ -50,7 +51,19 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule {
        // Origin defaults to users with sitewide authority
        protected $origin = self::ORIGIN_USER_SITEWIDE;
 
-       // In-process cache for title info
+       // In-process cache for title info, structured as an array
+       // [
+       //  <batchKey> // Pipe-separated list of sorted keys from getPages
+       //   => [
+       //     <titleKey> => [ // Normalised title key
+       //       'page_len' => ..,
+       //       'page_latest' => ..,
+       //       'page_touched' => ..,
+       //     ]
+       //   ]
+       // ]
+       // @see self::fetchTitleInfo()
+       // @see self::makeTitleKey()
        protected $titleInfo = [];
 
        // List of page names that contain CSS
@@ -295,8 +308,13 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule {
                return count( $revisions ) === 0;
        }
 
-       private function setTitleInfo( $key, array $titleInfo ) {
-               $this->titleInfo[$key] = $titleInfo;
+       private function setTitleInfo( $batchKey, array $titleInfo ) {
+               $this->titleInfo[$batchKey] = $titleInfo;
+       }
+
+       private static function makeTitleKey( LinkTarget $title ) {
+               // Used for keys in titleInfo.
+               return "{$title->getNamespace()}:{$title->getDBkey()}";
        }
 
        /**
@@ -313,11 +331,11 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule {
 
                $pageNames = array_keys( $this->getPages( $context ) );
                sort( $pageNames );
-               $key = implode( '|', $pageNames );
-               if ( !isset( $this->titleInfo[$key] ) ) {
-                       $this->titleInfo[$key] = static::fetchTitleInfo( $dbr, $pageNames, __METHOD__ );
+               $batchKey = implode( '|', $pageNames );
+               if ( !isset( $this->titleInfo[$batchKey] ) ) {
+                       $this->titleInfo[$batchKey] = static::fetchTitleInfo( $dbr, $pageNames, __METHOD__ );
                }
-               return $this->titleInfo[$key];
+               return $this->titleInfo[$batchKey];
        }
 
        protected static function fetchTitleInfo( IDatabase $db, array $pages, $fname = __METHOD__ ) {
@@ -340,8 +358,8 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule {
                        foreach ( $res as $row ) {
                                // Avoid including ids or timestamps of revision/page tables so
                                // that versions are not wasted
-                               $title = Title::makeTitle( $row->page_namespace, $row->page_title );
-                               $titleInfo[$title->getPrefixedText()] = [
+                               $title = new TitleValue( (int)$row->page_namespace, $row->page_title );
+                               $titleInfo[ self::makeTitleKey( $title ) ] = [
                                        'page_len' => $row->page_len,
                                        'page_latest' => $row->page_latest,
                                        'page_touched' => $row->page_touched,
@@ -410,23 +428,23 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule {
                        $pages = $wikiModule->getPages( $context );
                        // Before we intersect, map the names to canonical form (T145673).
                        $intersect = [];
-                       foreach ( $pages as $page => $unused ) {
-                               $title = Title::newFromText( $page );
+                       foreach ( $pages as $pageName => $unused ) {
+                               $title = Title::newFromText( $pageName );
                                if ( $title ) {
-                                       $intersect[ $title->getPrefixedText() ] = 1;
+                                       $intersect[ self::makeTitleKey( $title ) ] = 1;
                                } else {
                                        // Page name may be invalid if user-provided (e.g. gadgets)
                                        $rl->getLogger()->info(
                                                'Invalid wiki page title "{title}" in ' . __METHOD__,
-                                               [ 'title' => $page ]
+                                               [ 'title' => $pageName ]
                                        );
                                }
                        }
                        $info = array_intersect_key( $allInfo, $intersect );
                        $pageNames = array_keys( $pages );
                        sort( $pageNames );
-                       $key = implode( '|', $pageNames );
-                       $wikiModule->setTitleInfo( $key, $info );
+                       $batchKey = implode( '|', $pageNames );
+                       $wikiModule->setTitleInfo( $batchKey, $info );
                }
        }
 

diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php
index 0aa37d2..d4b9c16 100644 (file)
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php
@@ -15,6 +15,15 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase {
                $this->assertInstanceOf( ResourceLoaderWikiModule::class, $module );
        }
 
+       private function prepareTitleInfo( array $mockInfo ) {
+               $module = TestingAccessWrapper::newFromClass( ResourceLoaderWikiModule::class );
+               $info = [];
+               foreach ( $mockInfo as $key => $val ) {
+                       $info[ $module->makeTitleKey( Title::newFromText( $key ) ) ] = $val;
+               }
+               return $info;
+       }
+
        public static function provideConstructor() {
                return [
                        // Nothing
@@ -102,7 +111,7 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase {
                        ->getMock();
                $module->expects( $this->any() )
                        ->method( 'getTitleInfo' )
-                       ->will( $this->returnValue( $titleInfo ) );
+                       ->will( $this->returnValue( $this->prepareTitleInfo( $titleInfo ) ) );
                $module->expects( $this->any() )
                        ->method( 'getGroup' )
                        ->will( $this->returnValue( $group ) );
@@ -151,10 +160,10 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase {
                        'MediaWiki:Common.css' => [ 'type' => 'styles' ],
                        'mediawiki: fallback.css' => [ 'type' => 'styles' ],
                ];
-               $titleInfo = [
+               $titleInfo = $this->prepareTitleInfo( [
                        'MediaWiki:Common.css' => [ 'page_len' => 1234 ],
                        'MediaWiki:Fallback.css' => [ 'page_len' => 0 ],
-               ];
+               ] );
                $expected = $titleInfo;
 
                $module = $this->getMockBuilder( TestResourceLoaderWikiModule::class )
@@ -186,10 +195,10 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase {
                        // doing an intersect on the canonical result, producing an empty array.
                        'mediawiki: fallback.css' => [ 'type' => 'styles' ],
                ];
-               $titleInfo = [
+               $titleInfo = $this->prepareTitleInfo( [
                        'MediaWiki:Common.css' => [ 'page_len' => 1234 ],
                        'MediaWiki:Fallback.css' => [ 'page_len' => 0 ],
-               ];
+               ] );
                $expected = $titleInfo;
 
                $module = $this->getMockBuilder( TestResourceLoaderWikiModule::class )