Fix ApiStashEdit wrt custom DataUpdates.

My previous patch broke this: ApiStashEdit would stash ParserOutput
with no custom DataUpdates, but calling getSecondaryDataUpdates still
failed after unserialization. This patch should fix that.

Bug: T86305
Change-Id: Ic114e521c5dfd0d3c028ea7d16e93eace758deef
This commit is contained in:
daniel 2015-01-09 19:16:00 +00:00
parent 213c71b55e
commit f10b8df598
3 changed files with 91 additions and 10 deletions

View file

@ -335,10 +335,8 @@ class ApiStashEdit extends ApiBase {
$ttl = min( $parserOutput->getCacheExpiry() - $since, 5 * 60 );
// Note: ParserOutput with that contains secondary data update callbacks can not be
// stashed, since the callbacks are not serializable (see ParserOtput::__sleep).
// The first data update returned by getSecondaryDataUpdates() is always a LinksUpdate
// instance generated on the fly, so it can be ignored in this context.
$hasCustomDataUpdates = count( $parserOutput->getSecondaryDataUpdates() ) > 1;
// stashed, since the callbacks are not serializable (see ParserOutput::__sleep).
$hasCustomDataUpdates = $parserOutput->hasCustomDataUpdates();
if ( $ttl > 0 && !$parserOutput->getFlag( 'vary-revision' ) && !$hasCustomDataUpdates ) {
// Only store what is actually needed

View file

@ -53,7 +53,8 @@ class ParserOutput extends CacheTime {
$mTOCEnabled = true; # Whether TOC should be shown, can't override __NOTOC__
private $mIndexPolicy = ''; # 'index' or 'noindex'? Any other value will result in no change.
private $mAccessedOptions = array(); # List of ParserOptions (stored in the keys)
private $mSecondaryDataUpdates = null; # List of DataUpdate, used to save info from the page somewhere else.
private $mSecondaryDataUpdates = array(); # List of DataUpdate, used to save info from the page somewhere else.
private $mCustomDataUpdateCount = 0; # Number of custom updaters in $mSecondaryDataUpdates.
private $mExtensionData = array(); # extra data used by extensions
private $mLimitReportData = array(); # Parser limit report data
private $mParseStartTime = array(); # Timestamps for getTimeSinceStart()
@ -70,8 +71,6 @@ class ParserOutput extends CacheTime {
$this->mCategories = $categoryLinks;
$this->mContainsOldMagic = $containsOldMagic;
$this->mTitleText = $titletext;
$this->mSecondaryDataUpdates = array();
}
public function getText() {
@ -682,12 +681,30 @@ class ParserOutput extends CacheTime {
* from the page's content. This is triggered by calling getSecondaryDataUpdates()
* and is used for forward links updates on edit and backlink updates by jobs.
*
* @note: custom DataUpdates do not survive serialization of the ParserOutput!
* This is especially relevant when using a cached ParserOutput for updating
* the database, as WikiPage does if $wgAjaxStashEdit is enabled. For this
* reason, ApiStashEdit will skip any ParserOutput that has custom DataUpdates.
*
* @since 1.20
*
* @param DataUpdate $update
*/
public function addSecondaryDataUpdate( DataUpdate $update ) {
$this->mSecondaryDataUpdates[] = $update;
$this->mCustomDataUpdateCount = count( $this->mSecondaryDataUpdates );
}
/**
* Whether this ParserOutput contains custom DataUpdate objects that may not survive
* serialization of the ParserOutput.
*
* @see __sleep()
*
* @return bool
*/
public function hasCustomDataUpdates() {
return ( $this->mCustomDataUpdateCount > 0 );
}
/**
@ -714,10 +731,10 @@ class ParserOutput extends CacheTime {
$title = Title::newFromText( $this->getTitleText() );
}
if ( $this->mSecondaryDataUpdates === null ) {
//NOTE: This happens when mSecondaryDataUpdates are lost during serialization
if ( count( $this->mSecondaryDataUpdates ) !== $this->mCustomDataUpdateCount ) {
// NOTE: This happens when mSecondaryDataUpdates are lost during serialization
// (see __sleep below). After (un)serialization, getSecondaryDataUpdates()
// has no defined behavior, and should throw an exception.
// has no defined behavior in that case, and should throw an exception.
throw new MWException( 'getSecondaryDataUpdates() must not be called on ParserOutput restored from serialization.' );
}

View file

@ -1,5 +1,9 @@
<?php
/**
* @group Database
* ^--- trigger DB shadowing because we are using Title magic
*/
class ParserOutputTest extends MediaWikiTestCase {
public static function provideIsLinkInternal() {
@ -84,4 +88,66 @@ class ParserOutputTest extends MediaWikiTestCase {
$this->assertEquals( $po->getProperty( 'foo' ), false );
$this->assertArrayNotHasKey( 'foo', $properties );
}
/**
* @covers ParserOutput::hasCustomDataUpdates
* @covers ParserOutput::addSecondaryDataUpdate
*/
public function testHasCustomDataUpdates() {
$po = new ParserOutput();
$this->assertFalse( $po->hasCustomDataUpdates() );
$dataUpdate = $this->getMock( 'DataUpdate' );
$po->addSecondaryDataUpdate( $dataUpdate );
$this->assertTrue( $po->hasCustomDataUpdates() );
}
/**
* @covers ParserOutput::getSecondaryDataUpdate
* @covers ParserOutput::addSecondaryDataUpdate
*/
public function testGetSecondaryDataUpdates() {
// NOTE: getSecondaryDataUpdates always returns a LinksUpdate object
// in addition to the DataUpdates registered via addSecondaryDataUpdate().
$title = Title::makeTitle( NS_MAIN, 'Dummy' );
$title->resetArticleID( 7777777 );
$po = new ParserOutput();
$this->assertCount( 1, $po->getSecondaryDataUpdates( $title ) );
$dataUpdate = $this->getMock( 'DataUpdate' );
$po->addSecondaryDataUpdate( $dataUpdate );
$this->assertCount( 2, $po->getSecondaryDataUpdates( $title ) );
// Test Fallback to getTitleText
$this->insertPage( 'Project:ParserOutputTestDummyPage' );
$po->setTitleText( 'Project:ParserOutputTestDummyPage' );
$this->assertCount( 2, $po->getSecondaryDataUpdates() );
}
/**
* @covers ParserOutput::getSecondaryDataUpdate
* @covers ParserOutput::__sleep
*/
public function testGetSecondaryDataUpdates_serialization() {
$title = Title::makeTitle( NS_MAIN, 'Dummy' );
$title->resetArticleID( 7777777 );
$po = new ParserOutput();
// Serializing is fine with no custom DataUpdates.
$po = unserialize( serialize( $po ) );
$this->assertCount( 1, $po->getSecondaryDataUpdates( $title ) );
// If there are custom DataUpdates, getSecondaryDataUpdates
// should fail after serialization.
$dataUpdate = $this->getMock( 'DataUpdate' );
$po->addSecondaryDataUpdate( $dataUpdate );
$po = unserialize( serialize( $po ) );
$this->setExpectedException( 'MWException' );
$po->getSecondaryDataUpdates( $title );
}
}