Use ParserCache in CategoryMembershipChangeJob
authordaniel <dkinzler@wikimedia.org>
Wed, 14 Nov 2018 17:07:20 +0000 (18:07 +0100)
committerdaniel <dkinzler@wikimedia.org>
Tue, 27 Nov 2018 23:46:14 +0000 (15:46 -0800)
Note that we will still be re-parsing either the old or the new
revision. Keeping the rendered version of the old revision cached
for a bit would be nice, but ParserCache currently does not
support this.

Bug: T205369
Change-Id: I86d26e494924eec24e7b1fb32c424ac1284be478

includes/DefaultSettings.php
includes/Storage/DerivedPageDataUpdater.php
includes/jobqueue/jobs/CategoryMembershipChangeJob.php
tests/phpunit/includes/jobqueue/jobs/CategoryMembershipChangeJobTest.php

index 2d1681c..588d02f 100644 (file)
@@ -37,6 +37,7 @@
  *
  * @file
  */
+use MediaWiki\MediaWikiServices;
 
 /**
  * @cond file_level_code
@@ -7557,7 +7558,10 @@ $wgJobClasses = [
        'refreshLinksPrioritized' => RefreshLinksJob::class,
        'refreshLinksDynamic' => RefreshLinksJob::class,
        'activityUpdateJob' => ActivityUpdateJob::class,
-       'categoryMembershipChange' => CategoryMembershipChangeJob::class,
+       'categoryMembershipChange' => function ( Title $title, $params = [] ) {
+               $pc = MediaWikiServices::getInstance()->getParserCache();
+               return new CategoryMembershipChangeJob( $pc, $title, $params );
+       },
        'clearUserWatchlist' => ClearUserWatchlistJob::class,
        'cdnPurge' => CdnPurgeJob::class,
        'userGroupExpiry' => UserGroupExpiryJob::class,
index ad29f91..8e4f90d 100644 (file)
@@ -1407,12 +1407,9 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                        // the recent change entry (also done via deferred updates) and carry over any
                        // bot/deletion/IP flags, ect.
                        $this->jobQueueGroup->lazyPush(
-                               new CategoryMembershipChangeJob(
+                               CategoryMembershipChangeJob::newSpec(
                                        $this->getTitle(),
-                                       [
-                                               'pageId' => $this->getPageId(),
-                                               'revTimestamp' => $this->revision->getTimestamp(),
-                                       ]
+                                       $this->revision->getTimestamp()
                                )
                        );
                }
index c39823f..1c7647c 100644 (file)
@@ -41,11 +41,38 @@ class CategoryMembershipChangeJob extends Job {
 
        const ENQUEUE_FUDGE_SEC = 60;
 
-       public function __construct( Title $title, array $params ) {
+       /**
+        * @var ParserCache
+        */
+       private $parserCache;
+
+       /**
+        * @param Title $title The title of the page for which to update category emmbership.
+        * @param string $revisionTimestamp The timestamp of the new revision that triggered the job.
+        * @return JobSpecification
+        */
+       public static function newSpec( Title $title, $revisionTimestamp ) {
+               return new JobSpecification(
+                       'categoryMembershipChange',
+                       [
+                               'pageId' => $title->getArticleID(),
+                               'revTimestamp' => $revisionTimestamp,
+                       ],
+                       [],
+                       $title
+               );
+       }
+
+       /**
+        * Constructor for use by the Job Queue infrastructure.
+        * @note Don't call this when queueing a new instance, use newSpec() instead.
+        */
+       public function __construct( ParserCache $parserCache, Title $title, array $params ) {
                parent::__construct( 'categoryMembershipChange', $title, $params );
                // Only need one job per page. Note that ENQUEUE_FUDGE_SEC handles races where an
                // older revision job gets inserted while the newer revision job is de-duplicated.
                $this->removeDuplicates = true;
+               $this->parserCache = $parserCache;
        }
 
        public function run() {
@@ -236,10 +263,12 @@ class CategoryMembershipChangeJob extends Job {
                $options = $page->makeParserOptions( 'canonical' );
                $options->setTimestamp( $parseTimestamp );
 
-               // This could possibly use the parser cache if it checked the revision ID,
-               // but that's more complicated than it's worth.
-               $output = $renderer->getRenderedRevision( $rev->getRevisionRecord(), $options )
-                       ->getRevisionParserOutput();
+               $output = $rev->isCurrent() ? $this->parserCache->get( $page, $options ) : null;
+
+               if ( !$output || $output->getCacheRevisionId() !== $rev->getId() ) {
+                       $output = $renderer->getRenderedRevision( $rev->getRevisionRecord(), $options )
+                               ->getRevisionParserOutput();
+               }
 
                // array keys will cast numeric category names to ints
                // so we need to cast them back to strings to avoid breaking things!
index 1f73324..9f88cd7 100644 (file)
@@ -61,13 +61,16 @@ class CategoryMembershipChangeJobTest extends MediaWikiTestCase {
         * @return RecentChange|null
         */
        private function getCategorizeRecentChangeForRevId( $revId ) {
-               return RecentChange::newFromConds(
+               $rc = RecentChange::newFromConds(
                        [
                                'rc_type' => RC_CATEGORIZE,
                                'rc_this_oldid' => $revId,
                        ],
                        __METHOD__
                );
+
+               $this->assertNotNull( $rc, 'rev__id = ' . $revId );
+               return $rc;
        }
 
        public function testRun_normalCategoryAddedAndRemoved() {