Move "dirty" logic to PoolWorkArticleView subclass that uses it
There is only a single subclass that ever does anything with these two boolean properties. Only 3 states are possible. Pretty much all of this belongs to the subclass. No other code should have to know anything about this. This patch doesn't fully solve the issue but moves code in the described direction. Bug: T304813 Change-Id: I70754546f065b03ff04a73307c10f22fbb040810
This commit is contained in:
parent
0a83300357
commit
dfbf5830b2
4 changed files with 21 additions and 38 deletions
|
|
@ -309,11 +309,10 @@ class ParserOutputAccess {
|
|||
$classCacheKey = $this->primaryCache->makeParserOutputKey( $page, $parserOptions );
|
||||
$this->localCache[$classCacheKey] = $output;
|
||||
}
|
||||
if ( $work->getIsDirty() ) {
|
||||
$staleReason = $work->getIsFastStale()
|
||||
? 'view-pool-contention' : 'view-pool-overload';
|
||||
if ( $work instanceof PoolWorkArticleViewCurrent && $work->getDirtyWarning() ) {
|
||||
$status->warning( 'view-pool-dirty-output' );
|
||||
$status->warning( $staleReason );
|
||||
// @phan-suppress-next-line PhanTypeMismatchArgumentNullable Null check is above
|
||||
$status->warning( $work->getDirtyWarning() );
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -45,12 +45,6 @@ class PoolWorkArticleView extends PoolCounterWork {
|
|||
/** @var ParserOutput|bool */
|
||||
protected $parserOutput = false;
|
||||
|
||||
/** @var bool */
|
||||
protected $isDirty = false;
|
||||
|
||||
/** @var bool */
|
||||
protected $isFast = false;
|
||||
|
||||
/** @var Status|bool */
|
||||
protected $error = false;
|
||||
|
||||
|
|
@ -87,24 +81,6 @@ class PoolWorkArticleView extends PoolCounterWork {
|
|||
return $this->parserOutput;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get whether the ParserOutput is a dirty one (i.e. expired)
|
||||
*
|
||||
* @return bool
|
||||
*/
|
||||
public function getIsDirty() {
|
||||
return $this->isDirty;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get whether the ParserOutput was retrieved in fast stale mode
|
||||
*
|
||||
* @return bool
|
||||
*/
|
||||
public function getIsFastStale() {
|
||||
return $this->isFast;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get a Status object in case of error or false otherwise
|
||||
*
|
||||
|
|
|
|||
|
|
@ -47,6 +47,9 @@ class PoolWorkArticleViewCurrent extends PoolWorkArticleView {
|
|||
/** @var WikiPageFactory */
|
||||
private $wikiPageFactory;
|
||||
|
||||
/** @var string|null */
|
||||
private $dirtyWarning = null;
|
||||
|
||||
/**
|
||||
* @param string $workKey
|
||||
* @param PageRecord $page
|
||||
|
|
@ -170,8 +173,17 @@ class PoolWorkArticleViewCurrent extends PoolWorkArticleView {
|
|||
}
|
||||
|
||||
$logger->info( $fast ? 'fast dirty output' : 'dirty output', [ 'workKey' => $this->workKey ] );
|
||||
$this->isDirty = true;
|
||||
$this->isFast = $fast;
|
||||
$this->dirtyWarning = $fast ? 'view-pool-contention' : 'view-pool-overload';
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get whether the ParserOutput is a dirty one (i.e. expired)
|
||||
*
|
||||
* @return string|null
|
||||
*/
|
||||
public function getDirtyWarning(): ?string {
|
||||
return $this->dirtyWarning;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -129,13 +129,11 @@ class PoolWorkArticleViewCurrentTest extends PoolWorkArticleViewTest {
|
|||
|
||||
$this->assertFalse( $work->fallback( true ) );
|
||||
$this->assertFalse( $work->getParserOutput() );
|
||||
$this->assertFalse( $work->getIsDirty() );
|
||||
$this->assertFalse( $work->getIsFastStale() );
|
||||
$this->assertNull( $work->getDirtyWarning() );
|
||||
|
||||
$this->assertTrue( $work->fallback( false ) );
|
||||
$this->assertInstanceOf( ParserOutput::class, $work->getParserOutput() );
|
||||
$this->assertTrue( $work->getIsDirty() );
|
||||
$this->assertFalse( $work->getIsFastStale() );
|
||||
$this->assertSame( 'view-pool-overload', $work->getDirtyWarning() );
|
||||
}
|
||||
|
||||
public function testFallbackFromMoreRecentParserCache() {
|
||||
|
|
@ -159,13 +157,11 @@ class PoolWorkArticleViewCurrentTest extends PoolWorkArticleViewTest {
|
|||
|
||||
$this->assertTrue( $work->fallback( true ) );
|
||||
$this->assertInstanceOf( ParserOutput::class, $work->getParserOutput() );
|
||||
$this->assertTrue( $work->getIsDirty() );
|
||||
$this->assertTrue( $work->getIsFastStale() );
|
||||
$this->assertSame( 'view-pool-contention', $work->getDirtyWarning() );
|
||||
|
||||
$this->assertTrue( $work->fallback( false ) );
|
||||
$this->assertInstanceOf( ParserOutput::class, $work->getParserOutput() );
|
||||
$this->assertTrue( $work->getIsDirty() );
|
||||
$this->assertFalse( $work->getIsFastStale() );
|
||||
$this->assertSame( 'view-pool-overload', $work->getDirtyWarning() );
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue