From: daniel Date: Fri, 9 Jan 2015 19:16:00 +0000 (+0000) Subject: Fix ApiStashEdit wrt custom DataUpdates. X-Git-Tag: 1.31.0-rc.0~12731^2 X-Git-Url: http://git.cyclocoop.org/?a=commitdiff_plain;h=f10b8df598ff245ba3d1b19e63c99fb31d01b973;p=lhc%2Fweb%2Fwiklou.git 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 --- diff --git a/includes/api/ApiStashEdit.php b/includes/api/ApiStashEdit.php index 4d181d61a4..09489e4f9a 100644 --- a/includes/api/ApiStashEdit.php +++ b/includes/api/ApiStashEdit.php @@ -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 diff --git a/includes/parser/ParserOutput.php b/includes/parser/ParserOutput.php index f155312a85..117e04a122 100644 --- a/includes/parser/ParserOutput.php +++ b/includes/parser/ParserOutput.php @@ -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.' ); } diff --git a/tests/phpunit/includes/parser/ParserOutputTest.php b/tests/phpunit/includes/parser/ParserOutputTest.php index c024cee545..a6b4880dcd 100644 --- a/tests/phpunit/includes/parser/ParserOutputTest.php +++ b/tests/phpunit/includes/parser/ParserOutputTest.php @@ -1,5 +1,9 @@ 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 ); + } + }