resourceloader: Simplify WikiModule::getTitleInfo() DB query
authorTimo Tijhof <krinklemail@gmail.com>
Thu, 8 Sep 2016 02:24:49 +0000 (19:24 -0700)
committerKrinkle <krinklemail@gmail.com>
Thu, 8 Sep 2016 03:29:58 +0000 (03:29 +0000)
While rev_sha1 was preferred to rev_id (page_latest) to allow
client-cache to be re-used in case of a bad change and revert,
this is no longer possible since we recently added support for
explicit purging by considering page_touched.

As such, use of rev_sha1 is no longer useful.

Change-Id: Iddb65305ca4655098fdea9fcf736fd4046035dd7

includes/resourceloader/ResourceLoaderWikiModule.php
tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php

index 390f785..8033843 100644 (file)
@@ -246,7 +246,7 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule {
                $summary = parent::getDefinitionSummary( $context );
                $summary[] = [
                        'pages' => $this->getPages( $context ),
-                       // Includes SHA1 of content
+                       // Includes meta data of current revisions
                        'titleInfo' => $this->getTitleInfo( $context ),
                ];
                return $summary;
@@ -262,7 +262,7 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule {
                // For user modules, don't needlessly load if there are no non-empty pages
                if ( $this->getGroup() === 'user' ) {
                        foreach ( $revisions as $revision ) {
-                               if ( $revision['rev_len'] > 0 ) {
+                               if ( $revision['page_len'] > 0 ) {
                                        // At least one non-empty page, module should be loaded
                                        return false;
                                }
@@ -279,7 +279,7 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule {
        /**
         * Get the information about the wiki pages for a given context.
         * @param ResourceLoaderContext $context
-        * @return array Keyed by page name. Contains arrays with 'rev_len' and 'rev_sha1' keys
+        * @return array Keyed by page name
         */
        protected function getTitleInfo( ResourceLoaderContext $context ) {
                $dbr = $this->getDB();
@@ -298,21 +298,19 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule {
                        }
 
                        if ( !$batch->isEmpty() ) {
-                               $res = $dbr->select( [ 'page', 'revision' ],
+                               $res = $dbr->select( 'page',
                                        // Include page_touched to allow purging if cache is poisoned (T117587, T113916)
-                                       [ 'page_namespace', 'page_title', 'page_touched', 'rev_len', 'rev_sha1' ],
+                                       [ 'page_namespace', 'page_title', 'page_touched', 'page_len', 'page_latest' ],
                                        $batch->constructSet( 'page', $dbr ),
-                                       __METHOD__,
-                                       [],
-                                       [ 'revision' => [ 'INNER JOIN', [ 'page_latest=rev_id' ] ] ]
+                                       __METHOD__
                                );
                                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 );
                                        $this->titleInfo[$key][$title->getPrefixedText()] = [
-                                               'rev_len' => $row->rev_len,
-                                               'rev_sha1' => $row->rev_sha1,
+                                               'page_len' => $row->page_len,
+                                               'page_latest' => $row->page_latest,
                                                'page_touched' => $row->page_touched,
                                        ];
                                }
index 85834d7..404fd97 100644 (file)
@@ -114,25 +114,25 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase {
                        [ [], 'test1', true ],
                        // 'site' module with a non-empty page
                        [
-                               [ 'MediaWiki:Common.js' => [ 'rev_sha1' => 'dmh6qn', 'rev_len' => 1234 ] ],
+                               [ 'MediaWiki:Common.js' => [ 'page_len' => 1234 ] ],
                                'site',
                                false,
                        ],
                        // 'site' module with an empty page
                        [
-                               [ 'MediaWiki:Foo.js' => [ 'rev_sha1' => 'phoi', 'rev_len' => 0 ] ],
+                               [ 'MediaWiki:Foo.js' => [ 'page_len' => 0 ] ],
                                'site',
                                false,
                        ],
                        // 'user' module with a non-empty page
                        [
-                               [ 'User:Example/common.js' => [ 'rev_sha1' => 'j7ssba', 'rev_len' => 25 ] ],
+                               [ 'User:Example/common.js' => [ 'page_len' => 25 ] ],
                                'user',
                                false,
                        ],
                        // 'user' module with an empty page
                        [
-                               [ 'User:Example/foo.js' => [ 'rev_sha1' => 'phoi', 'rev_len' => 0 ] ],
+                               [ 'User:Example/foo.js' => [ 'page_len' => 0 ] ],
                                'user',
                                true,
                        ],