Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Extract PoolWorlArticleViewCurrent

Browse source 
Extracts a specialized subclass for rendering the current revision
from PoolWorlArticleView, which then no longer knowes about caching.

In the next step, we will add a subclass that implements caching for old
revisions.

Bug: T267832
Change-Id: I56fb365962951e6c723a01cf9243dbc0094b5581
This commit is contained in:
daniel 2020-11-16 21:05:03 +01:00
parent e6a6592ecf
commit ed41864370
9 changed files with 321 additions and 130 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
autoload.php
View file

@ -1260,6 +1260,7 @@ $wgAutoloadLocalClasses = [
'PoolCounterWork' => __DIR__ . '/includes/poolcounter/PoolCounterWork.php',
'PoolCounterWorkViaCallback' => __DIR__ . '/includes/poolcounter/PoolCounterWorkViaCallback.php',
'PoolWorkArticleView' => __DIR__ . '/includes/poolcounter/PoolWorkArticleView.php',
'PoolWorkArticleViewCurrent' => __DIR__ . '/includes/poolcounter/PoolWorkArticleViewCurrent.php',
'PopulateArchiveRevId' => __DIR__ . '/maintenance/populateArchiveRevId.php',
'PopulateBacklinkNamespace' => __DIR__ . '/maintenance/populateBacklinkNamespace.php',
'PopulateCategory' => __DIR__ . '/maintenance/populateCategory.php',

3
includes/ServiceWiring.php
View file

@ -947,7 +947,8 @@ return [
return new ParserOutputAccess(
$services->getParserCache(),
$services->getRevisionRenderer(),
$services->getStatsdDataFactory()
$services->getStatsdDataFactory(),
$services->getDBLoadBalancerFactory()
);
},

43
includes/page/ParserOutputAccess.php
View file

@ -29,7 +29,9 @@ use ParserCache;
use ParserOptions;
use ParserOutput;
use PoolWorkArticleView;
use PoolWorkArticleViewCurrent;
use Status;
use Wikimedia\Rdbms\ILBFactory;
use WikiPage;
/**
@ -79,19 +81,25 @@ class ParserOutputAccess {
/** @var IBufferingStatsdDataFactory */
private $statsDataFactory;
/** @var ILBFactory */
private $lbFactory;
/**
* @param ParserCache $primaryCache
* @param RevisionRenderer $revisionRenderer
* @param IBufferingStatsdDataFactory $statsDataFactory
* @param ILBFactory $lbFactory
*/
public function __construct(
ParserCache $primaryCache,
RevisionRenderer $revisionRenderer,
IBufferingStatsdDataFactory $statsDataFactory
IBufferingStatsdDataFactory $statsDataFactory,
ILBFactory $lbFactory
) {
$this->primaryCache = $primaryCache;
$this->revisionRenderer = $revisionRenderer;
$this->statsDataFactory = $statsDataFactory;
$this->lbFactory = $lbFactory;
}
/**
@ -307,15 +315,34 @@ class ParserOutputAccess {
$useCache = $this->shouldUseCache( $page, $parserOptions, $revision );
}
$work = new PoolWorkArticleView(
$page,
$revision,
$parserOptions,
$useCache,
$this->revisionRenderer,
$this->primaryCache
$parserCacheMetadata = $this->primaryCache->getMetadata( $page );
$cacheKey = $this->primaryCache->makeParserOutputKey( $page, $parserOptions,
$parserCacheMetadata ? $parserCacheMetadata->getUsedOptions() : null
);
$workKey = $cacheKey . ':revid:' . $revision->getId();
if ( $useCache ) {
$workKey .= ':current';
$work = new PoolWorkArticleViewCurrent(
$workKey,
$page,
$revision,
$parserOptions,
$this->revisionRenderer,
$this->primaryCache,
$this->lbFactory
);
} else {
$workKey .= ':uncached';
$work = new PoolWorkArticleView(
$workKey,
$revision,
$parserOptions,
$this->revisionRenderer
);
}
return $work;
}

135
includes/poolcounter/PoolWorkArticleView.php
View file

@ -18,81 +18,56 @@
* @file
*/
use MediaWiki\MediaWikiServices;
use MediaWiki\Revision\RevisionRecord;
use MediaWiki\Revision\RevisionRenderer;
/**
* PoolCounter protected work wrapping RenderedRevision->getRevisionParserOutput.
* Caching behavior may be defined by subclasses.
*
* @note No audience checks are applied.
*
* @internal
*/
class PoolWorkArticleView extends PoolCounterWork {
/** @var WikiPage */
private $page;
/** @var string */
private $cacheKey;
/** @var ParserCache */
private $parserCache;
/** @var ParserOptions */
private $parserOptions;
protected $parserOptions;
/** @var RevisionRecord|null */
private $revision = null;
protected $revision = null;
/** @var RevisionRenderer */
private $renderer = null;
protected $renderer = null;
/** @var ParserOutput|bool */
private $parserOutput = false;
protected $parserOutput = false;
/** @var bool */
private $isDirty = false;
protected $isDirty = false;
/** @var bool */
private $isFast = false;
protected $isFast = false;
/** @var Status|bool */
private $error = false;
protected $error = false;
/**
* @param WikiPage $page
* @param string $workKey
* @param RevisionRecord $revision Revision to render
* @param ParserOptions $parserOptions ParserOptions to use for the parse
* @param bool $useParserCache Whether to use the parser cache.
* @param RevisionRenderer $revisionRenderer
* @param ParserCache $parserCache
*/
public function __construct(
WikiPage $page,
string $workKey,
RevisionRecord $revision,
ParserOptions $parserOptions,
bool $useParserCache,
RevisionRenderer $revisionRenderer,
ParserCache $parserCache
RevisionRenderer $revisionRenderer
) {
// TODO: Remove support for partially initialized RevisionRecord instances once
// Article no longer uses fake revisions.
if ( $revision->getPageId() && $revision->getPageId() !== $page->getTitle()->getArticleID() ) {
throw new InvalidArgumentException( '$page parameter mismatches $revision parameter' );
}
$this->page = $page;
parent::__construct( 'ArticleView', $workKey );
$this->revision = $revision;
$this->parserOptions = $parserOptions;
$this->cacheable = $useParserCache;
$this->renderer = $revisionRenderer;
$this->parserCache = $parserCache;
$parserCacheMetadata = $this->parserCache->getMetadata( $page );
$this->cacheKey = $this->parserCache->makeParserOutputKey( $page, $parserOptions,
$parserCacheMetadata ? $parserCacheMetadata->getUsedOptions() : null
);
parent::__construct( 'ArticleView', $this->cacheKey . ':revid:' . $revision->getId() );
}
/**
@ -135,8 +110,6 @@ class PoolWorkArticleView extends PoolCounterWork {
* @return bool
*/
public function doWork() {
$isCurrent = $this->revision->getId() === $this->page->getLatest();
$renderedRevision = $this->renderer->getRenderedRevision(
$this->revision,
$this->parserOptions,
@ -162,86 +135,40 @@ class PoolWorkArticleView extends PoolCounterWork {
$logger = MediaWiki\Logger\LoggerFactory::getInstance( 'slow-parse' );
$logger->info( 'Parsing {title} was slow, took {time} seconds', [
'time' => number_format( $time, 2 ),
'title' => $this->page->getTitle()->getPrefixedDBkey(),
'ns' => $this->page->getTitle()->getNamespace(),
'title' => $this->revision->getPageAsLinkTarget()->getDBkey(),
'ns' => $this->revision->getPageAsLinkTarget()->getNamespace(),
'trigger' => 'view',
] );
}
if ( $this->cacheable && $this->parserOutput->isCacheable() && $isCurrent ) {
$this->parserCache->save(
$this->parserOutput,
$this->page,
$this->parserOptions,
$cacheTime,
$this->revision->getId()
);
if ( $this->cacheable && $this->parserOutput->isCacheable() ) {
$this->saveInCache( $this->parserOutput, $cacheTime );
}
if ( $isCurrent ) {
$this->page->triggerOpportunisticLinksUpdate( $this->parserOutput );
}
$this->afterWork( $this->parserOutput );
return true;
}
/**
* @return bool
* Place the output in the cache from which getCachedWork() will retrieve it.
* Will be called before saveInCache().
*
* @param ParserOutput $output
* @param string $cacheTime
*/
public function getCachedWork() {
$this->parserOutput = $this->parserCache->get( $this->page, $this->parserOptions );
if ( $this->parserOutput === false ) {
wfDebug( __METHOD__ . ": parser cache miss" );
return false;
} else {
wfDebug( __METHOD__ . ": parser cache hit" );
return true;
}
protected function saveInCache( ParserOutput $output, string $cacheTime ) {
// noop
}
/**
* @param bool $fast Fast stale request
* @return bool
* Subclasses may implement this to perform some action after the work of rendering is done.
* Will be called after saveInCache().
*
* @param ParserOutput $output
*/
public function fallback( $fast ) {
$this->parserOutput = $this->parserCache->getDirty( $this->page, $this->parserOptions );
$fastMsg = '';
if ( $this->parserOutput && $fast ) {
/* Check if the stale response is from before the last write to the
* DB by this user. Declining to return a stale response in this
* case ensures that the user will see their own edit after page
* save.
*
* Note that the CP touch time is the timestamp of the shutdown of
* the save request, so there is a bias towards avoiding fast stale
* responses of potentially several seconds.
*/
$lastWriteTime = MediaWikiServices::getInstance()->getDBLoadBalancerFactory()
->getChronologyProtectorTouched();
$cacheTime = MWTimestamp::convert( TS_UNIX, $this->parserOutput->getCacheTime() );
if ( $lastWriteTime && $cacheTime <= $lastWriteTime ) {
wfDebugLog( 'dirty', "declining to send dirty output since cache time " .
$cacheTime . " is before last write time $lastWriteTime" );
// Forget this ParserOutput -- we will request it again if
// necessary in slow mode. There might be a newer entry
// available by that time.
$this->parserOutput = false;
return false;
}
$this->isFast = true;
$fastMsg = 'fast ';
}
if ( $this->parserOutput === false ) {
wfDebugLog( 'dirty', 'dirty missing' );
return false;
} else {
wfDebugLog( 'dirty', "{$fastMsg}dirty output {$this->cacheKey}" );
$this->isDirty = true;
return true;
}
protected function afterWork( ParserOutput $output ) {
// noop
}
/**

155
includes/poolcounter/PoolWorkArticleViewCurrent.php Normal file
View file

@ -0,0 +1,155 @@
<?php
/**
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
* http://www.gnu.org/copyleft/gpl.html
*
* @file
*/
use MediaWiki\Revision\RevisionRecord;
use MediaWiki\Revision\RevisionRenderer;
use Wikimedia\Rdbms\ILBFactory;
/**
* PoolWorkArticleView for the current revision of a page, using ParserCache.
*
* @internal
*/
class PoolWorkArticleViewCurrent extends PoolWorkArticleView {
/** @var string */
private $workKey;
/** @var WikiPage */
private $page;
/** @var ParserCache */
private $parserCache;
/** @var ILBFactory */
private $lbFactory;
/**
* @param string $workKey
* @param WikiPage $page
* @param RevisionRecord $revision Revision to render
* @param ParserOptions $parserOptions ParserOptions to use for the parse
* @param RevisionRenderer $revisionRenderer
* @param ParserCache $parserCache
* @param ILBFactory $lbFactory
*/
public function __construct(
string $workKey,
WikiPage $page,
RevisionRecord $revision,
ParserOptions $parserOptions,
RevisionRenderer $revisionRenderer,
ParserCache $parserCache,
ILBFactory $lbFactory
) {
// TODO: Remove support for partially initialized RevisionRecord instances once
// Article no longer uses fake revisions.
if ( $revision->getPageId() && $revision->getPageId() !== $page->getTitle()->getArticleID() ) {
throw new InvalidArgumentException( '$page parameter mismatches $revision parameter' );
}
parent::__construct( $workKey, $revision, $parserOptions, $revisionRenderer );
$this->workKey = $workKey;
$this->page = $page;
$this->parserCache = $parserCache;
$this->lbFactory = $lbFactory;
$this->cacheable = true;
}
/**
* @param ParserOutput $output
* @param string $cacheTime
*/
protected function saveInCache( ParserOutput $output, string $cacheTime ) {
$this->parserCache->save(
$output,
$this->page,
$this->parserOptions,
$cacheTime,
$this->revision->getId()
);
}
/**
* @param ParserOutput $output
*/
protected function afterWork( ParserOutput $output ) {
$this->page->triggerOpportunisticLinksUpdate( $this->parserOutput );
}
/**
* @return bool
*/
public function getCachedWork() {
$this->parserOutput = $this->parserCache->get( $this->page, $this->parserOptions );
if ( $this->parserOutput === false ) {
wfDebug( __METHOD__ . ": parser cache miss" );
return false;
} else {
wfDebug( __METHOD__ . ": parser cache hit" );
return true;
}
}
/**
* @param bool $fast Fast stale request
* @return bool
*/
public function fallback( $fast ) {
$this->parserOutput = $this->parserCache->getDirty( $this->page, $this->parserOptions );
$fastMsg = '';
if ( $this->parserOutput && $fast ) {
/* Check if the stale response is from before the last write to the
* DB by this user. Declining to return a stale response in this
* case ensures that the user will see their own edit after page
* save.
*
* Note that the CP touch time is the timestamp of the shutdown of
* the save request, so there is a bias towards avoiding fast stale
* responses of potentially several seconds.
*/
$lastWriteTime = $this->lbFactory->getChronologyProtectorTouched();
$cacheTime = MWTimestamp::convert( TS_UNIX, $this->parserOutput->getCacheTime() );
if ( $lastWriteTime && $cacheTime <= $lastWriteTime ) {
wfDebugLog( 'dirty', "declining to send dirty output since cache time " .
$cacheTime . " is before last write time $lastWriteTime" );
// Forget this ParserOutput -- we will request it again if
// necessary in slow mode. There might be a newer entry
// available by that time.
$this->parserOutput = false;
return false;
}
$this->isFast = true;
$fastMsg = 'fast ';
}
if ( $this->parserOutput === false ) {
wfDebugLog( 'dirty', 'dirty missing' );
return false;
} else {
wfDebugLog( 'dirty', "{$fastMsg}dirty output {$this->workKey}" );
$this->isDirty = true;
return true;
}
}
}

4
tests/common/TestsAutoLoader.php
View file

@ -145,6 +145,10 @@ $wgAutoloadClasses += [
'Wikimedia\Tests\SerializationTestUtils' =>
"$testDir/phpunit/includes/libs/serialization/SerializationTestUtils.php",
# tests/phpunit/includes/poolcounter
'PoolWorkArticleViewTest' =>
"$testDir/phpunit/includes/poolcounter/PoolWorkArticleViewTest.php",
# tests/phpunit/includes/resourceloader
'ResourceLoaderImageModuleTest' =>
"$testDir/phpunit/includes/resourceloader/ResourceLoaderImageModuleTest.php",

3
tests/phpunit/includes/page/ParserOutputAccessTest.php
View file

@ -85,8 +85,9 @@ class ParserOutputAccessTest extends MediaWikiIntegrationTestCase {
}
$revRenderer = $this->getServiceContainer()->getRevisionRenderer();
$lbFactory = $this->getServiceContainer()->getDBLoadBalancerFactory();
$stats = new NullStatsdDataFactory();
return new ParserOutputAccess( $parserCache, $revRenderer, $stats );
return new ParserOutputAccess( $parserCache, $revRenderer, $stats, $lbFactory );
}
/**

80
tests/phpunit/includes/poolcounter/PoolWorkArticleViewCurrentTest.php Normal file
View file

@ -0,0 +1,80 @@
<?php
use MediaWiki\Json\JsonUnserializer;
use MediaWiki\Revision\RevisionRecord;
use Psr\Log\NullLogger;
/**
* @covers PoolWorkArticleViewCurrent
* @group Database
*/
class PoolWorkArticleViewCurrentTest extends PoolWorkArticleViewTest {
/** @var ParserCache */
private $parserCache = null;
/**
* @param WikiPage $page
* @param RevisionRecord|null $rev
* @param ParserOptions|null $options
*
* @return PoolWorkArticleView
*/
protected function newPoolWorkArticleView(
WikiPage $page,
RevisionRecord $rev = null,
$options = null
) {
if ( !$options ) {
$options = ParserOptions::newCanonical( 'canonical' );
}
if ( !$rev ) {
$rev = $page->getRevisionRecord();
}
$parserCache = $this->parserCache ?: $this->installParserCache();
$lbFactory = $this->getServiceContainer()->getDBLoadBalancerFactory();
$revisionRenderer = $this->getServiceContainer()->getRevisionRenderer();
return new PoolWorkArticleViewCurrent(
'test:' . $rev->getId(),
$page,
$rev,
$options,
$revisionRenderer,
$parserCache,
$lbFactory
);
}
private function installParserCache( $bag = null ) {
$this->parserCache = new ParserCache(
'test',
$bag ?: new HashBagOStuff(),
'',
$this->getServiceContainer()->getHookContainer(),
new JsonUnserializer(),
$this->getServiceContainer()->getStatsdDataFactory(),
new NullLogger()
);
return $this->parserCache;
}
public function testUpdateCachedOutput() {
$options = ParserOptions::newCanonical( 'canonical' );
$page = $this->getExistingTestPage( __METHOD__ );
$parserCache = $this->installParserCache();
// rendering of a deleted revision should work, audience checks are bypassed
$work = $this->newPoolWorkArticleView( $page, null, $options );
$this->assertTrue( $work->execute() );
$cachedOutput = $parserCache->get( $page, $options );
$this->assertNotEmpty( $cachedOutput );
$this->assertSame( $work->getParserOutput()->getText(), $cachedOutput->getText() );
}
}

27
tests/phpunit/includes/poolcounter/PoolWorkArticleViewTest.php
View file

@ -13,17 +13,15 @@ class PoolWorkArticleViewTest extends MediaWikiIntegrationTestCase {
/**
* @param WikiPage $page
* @param RevisionRecord $rev
* @param null $options
* @param bool $useParserCache
* @param RevisionRecord|null $rev
* @param ParserOptions|null $options
*
* @return PoolWorkArticleView
*/
private function newPoolWorkArticleView(
protected function newPoolWorkArticleView(
WikiPage $page,
RevisionRecord $rev = null,
$options = null,
$useParserCache = true
$options = null
) {
if ( !$options ) {
$options = ParserOptions::newCanonical( 'canonical' );
@ -33,16 +31,13 @@ class PoolWorkArticleViewTest extends MediaWikiIntegrationTestCase {
$rev = $page->getRevisionRecord();
}
$parserCache = $this->getServiceContainer()->getParserCache();
$revisionRenderer = $this->getServiceContainer()->getRevisionRenderer();
return new PoolWorkArticleView(
$page,
'test:' . $rev->getId(),
$rev,
$options,
$useParserCache,
$revisionRenderer,
$parserCache
$revisionRenderer
);
}
@ -60,11 +55,11 @@ class PoolWorkArticleViewTest extends MediaWikiIntegrationTestCase {
$rev1 = $this->makeRevision( $page, 'First!' );
$rev2 = $this->makeRevision( $page, 'Second!' );
$work = $this->newPoolWorkArticleView( $page, $rev1, $options, false );
$work = $this->newPoolWorkArticleView( $page, $rev1, $options );
$work->execute();
$this->assertStringContainsString( 'First', $work->getParserOutput()->getText() );
$work = $this->newPoolWorkArticleView( $page, $rev2, $options, false );
$work = $this->newPoolWorkArticleView( $page, $rev2, $options );
$work->execute();
$this->assertStringContainsString( 'Second', $work->getParserOutput()->getText() );
}
@ -74,7 +69,7 @@ class PoolWorkArticleViewTest extends MediaWikiIntegrationTestCase {
$page = $this->getExistingTestPage( __METHOD__ );
$rev1 = $this->makeRevision( $page, 'First!' );
$work = $this->newPoolWorkArticleView( $page, $rev1, $options, true );
$work = $this->newPoolWorkArticleView( $page, $rev1, $options );
$work->execute();
$cache = MediaWikiServices::getInstance()->getParserCache();
@ -95,7 +90,7 @@ class PoolWorkArticleViewTest extends MediaWikiIntegrationTestCase {
$fakeRev = new MutableRevisionRecord( $page->getTitle() );
$fakeRev->setContent( SlotRecord::MAIN, new WikitextContent( 'YES!' ) );
$work = $this->newPoolWorkArticleView( $page, $fakeRev, $options, false );
$work = $this->newPoolWorkArticleView( $page, $fakeRev, $options );
$work->execute();
$text = $work->getParserOutput()->getText();
@ -174,7 +169,7 @@ class PoolWorkArticleViewTest extends MediaWikiIntegrationTestCase {
$fakeRev->setVisibility( RevisionRecord::DELETED_TEXT );
// rendering of a deleted revision should work, audience checks are bypassed
$work = $this->newPoolWorkArticleView( $page, $fakeRev, $options, false );
$work = $this->newPoolWorkArticleView( $page, $fakeRev, $options );
$this->assertTrue( $work->execute() );
}