cleanup and fixes for secondary data updates

This commit is contained in:
daniel 2012-06-07 14:57:43 +02:00
parent a827739fb5
commit 48d0bedd78
10 changed files with 142 additions and 33 deletions

View file

@ -287,6 +287,9 @@ abstract class Content {
* Convenience method, shorthand for
* $this->getContentHandler()->getParserOutput( $this, $title, $revId, $options, $generateHtml )
*
* @note: subclasses should NOT override this to provide custom rendering.
* Override ContentHandler::getParserOutput() instead!
*
* @param Title $title
* @param null $revId
* @param null|ParserOptions $options
@ -302,23 +305,6 @@ abstract class Content {
return $this->getContentHandler()->getParserOutput( $this, $title, $revId, $options, $generateHtml );
}
/**
* Convenience method, shorthand for
* $this->getContentHandler()->getSecondaryDataUpdates( $this, $title, $old, $recursive )
*
* @param Title $title the context for determining the necessary updates
* @param Content|null $old a Content object representing the previous content, i.e. the content being
* replaced by this Content object.
* @param bool $recursive whether to include recursive updates (default: false).
*
* @return Array. A list of DataUpdate objects for putting information about this content object somewhere.
*
* @since WD.1
*/
public function getSecondaryDataUpdates( Title $title, Content $old = null, $recursive = false ) { #TODO: remove!
return $this->getContentHandler()->getSecondaryDataUpdates( $this, $title, $old, $recursive );
}
/**
* Construct the redirect destination from this content and return an
* array of Titles, or null if this content doesn't represent a redirect.

View file

@ -734,24 +734,32 @@ abstract class ContentHandler {
* data store. If the optional second argument, $old, is given, the updates may model only the changes that
* need to be made to replace information about the old content with information about the new content.
*
* This default implementation calls $this->getParserOutput( $title, null, null, false ), and then
* This default implementation calls $this->getParserOutput( $content, $title, null, null, false ), and then
* calls getSecondaryDataUpdates( $title, $recursive ) on the resulting ParserOutput object.
*
* Subclasses may implement this to determine the necessary updates more efficiently, or make use of information
* about the old content.
*
* @param Content $content the content for determining the necessary updates
* @param Title $title the context for determining the necessary updates
* @param Content|null $old a Content object representing the previous content, i.e. the content being
* @param Content|null $old an optional Content object representing the previous content, i.e. the content being
* replaced by this Content object.
* @param bool $recursive whether to include recursive updates (default: false).
* @param boolean $recursive whether to include recursive updates (default: false).
* @param ParserOutput|null $parserOutput optional ParserOutput object. Provide if you have one handy, to avoid re-parsing
* of the content.
*
* @return Array. A list of DataUpdate objects for putting information about this content object somewhere.
*
* @since WD.1
*/
public function getSecondaryDataUpdates( Content $content, Title $title, Content $old = null, $recursive = false ) {
$po = $this->getParserOutput( $content, $title, null, null, false );
return $po->getSecondaryDataUpdates( $title, $recursive );
public function getSecondaryDataUpdates( Content $content, Title $title, Content $old = null,
$recursive = true, ParserOutput $parserOutput = null ) {
if ( !$parserOutput ) {
$parserOutput = $this->getParserOutput( $content, $title, null, null, false );
}
return $parserOutput->getSecondaryDataUpdates( $title, $recursive );
}

View file

@ -63,7 +63,6 @@ class LinksUpdate extends SqlDataUpdate {
$this->mTitle = $title;
$this->mId = $title->getArticleID();
assert( $this->mId > 0 );
if ( !$this->mId ) {
throw new MWException( "The Title object did not provide an article ID. Perhaps the page doesn't exist?" );
@ -830,6 +829,10 @@ class LinksDeletionUpdate extends SqlDataUpdate {
parent::__construct( );
$this->mTitle = $title;
if ( !$title->getArticleID() ) {
throw new MWException( "The Title object did not provide an article ID. Perhaps the page doesn't exist?" );
}
}
/**

View file

@ -1204,6 +1204,10 @@ class Revision {
if ( $wgContentHandlerUseDB ) {
$row[ 'rev_content_model' ] = $this->getContentModel();
$row[ 'rev_content_format' ] = $this->getContentFormat();
} else {
// @todo: Make sure the revision is using the default model and format.
// Since we don't store the actual model and format, we won't have any way to determine it later
// if it's not the default.
}
$dbw->insert( 'revision', $row, __METHOD__ );

View file

@ -1983,7 +1983,8 @@ class WikiPage extends Page {
}
# Update the links tables and other secondary data
$updates = $editInfo->output->getSecondaryDataUpdates( $this->getTitle() ); #FIXME: call ContentHandler::getSecondaryLinkUpdates. Don't parse iuf not needed! But don't parse too early either, only after saving, so we have an article ID!
$contentHandler = $revision->getContentHandler();
$updates = $contentHandler->getSecondaryDataUpdates( $content, $this->getTitle(), null, true, $editInfo->output );
DataUpdate::runUpdates( $updates );
wfRunHooks( 'ArticleEditUpdates', array( &$this, &$editInfo, $options['changed'] ) );

View file

@ -94,7 +94,7 @@ class ApiPurge extends ApiBase {
true, true, $page->getLatest() ); #FIXME: content!
# Update the links tables
$updates = $p_result->getSecondaryDataUpdates( $title );
$updates = $p_result->getSecondaryDataUpdates( $title ); #FIXME: content handler
DataUpdate::runUpdates( $updates );
$r['linkupdate'] = '';

View file

@ -58,11 +58,11 @@ class RefreshLinksJob extends Job {
wfProfileIn( __METHOD__.'-parse' );
$options = ParserOptions::newFromUserAndLang( new User, $wgContLang );
$parserOutput = $wgParser->parse( $revision->getText(), $this->title, $options, true, true, $revision->getId() );
$parserOutput = $wgParser->parse( $revision->getText(), $this->title, $options, true, true, $revision->getId() ); #FIXME: content
wfProfileOut( __METHOD__.'-parse' );
wfProfileIn( __METHOD__.'-update' );
$updates = $parserOutput->getSecondaryDataUpdates( $this->title, false );
$updates = $parserOutput->getSecondaryDataUpdates( $this->title, false ); #FIXME: content handler
DataUpdate::runUpdates( $updates );
wfProfileOut( __METHOD__.'-update' );
@ -132,11 +132,11 @@ class RefreshLinksJob2 extends Job {
return false;
}
wfProfileIn( __METHOD__.'-parse' );
$parserOutput = $wgParser->parse( $revision->getText(), $title, $options, true, true, $revision->getId() );
$parserOutput = $wgParser->parse( $revision->getText(), $title, $options, true, true, $revision->getId() ); #FIXME: content handler
wfProfileOut( __METHOD__.'-parse' );
wfProfileIn( __METHOD__.'-update' );
$updates = $parserOutput->getSecondaryDataUpdates( $title, false );
$updates = $parserOutput->getSecondaryDataUpdates( $title, false ); #FIXME: content handler
DataUpdate::runUpdates( $updates );
wfProfileOut( __METHOD__.'-update' );

View file

@ -480,6 +480,9 @@ class ParserOutput extends CacheTime {
* extracted from the page's content, including a LinksUpdate object for all links stored in
* this ParserOutput object.
*
* @note: Avoid using this method directly, use ContentHandler::getSecondaryDataUpdates() instead! The content
* handler may provide additional update objects.
*
* @param $title Title of the page we're updating. If not given, a title object will be created based on $this->getTitleText()
* @param $recursive Boolean: queue jobs for recursive updates?
*

View file

@ -218,9 +218,8 @@ class RefreshLinks extends Maintenance {
$dbw = wfGetDB( DB_MASTER );
$dbw->begin( __METHOD__ );
$context = RequestContext::getMain();
$updates = $parserOutput->getSecondaryDataUpdates( $page->getTitle(), false );
$contentHandler = $content->getContentHandler();
$updates = $contentHandler->getSecondaryDataUpdates( $content, $page->getTitle() );
DataUpdate::runUpdates( $updates );
$dbw->commit( __METHOD__ );

View file

@ -243,7 +243,112 @@ class ContentHandlerTest extends MediaWikiTestCase {
}
public function dataGetParserOutput() {
return array(
array("ContentHandlerTest_testGetParserOutput", "hello ''world''\n", "<p>hello <i>world</i>\n</p>"),
// @todo: more...?
);
}
/**
* @dataProvider dataGetParserOutput
*/
public function testGetParserOutput( $title, $text, $expectedHtml ) {
$title = Title::newFromText( $title );
$handler = ContentHandler::getForModelID( $title->getContentModel() );
$content = ContentHandler::makeContent( $text, $title );
$po = $handler->getParserOutput( $content, $title );
$this->assertEquals( $expectedHtml, $po->getText() );
// @todo: assert more properties
}
public function dataGetSecondaryDataUpdates() {
return array(
array("ContentHandlerTest_testGetSecondaryDataUpdates_1", "hello ''world''\n",
array( 'LinksUpdate' => array( 'mRecursive' => true,
'mLinks' => array() ) )
),
array("ContentHandlerTest_testGetSecondaryDataUpdates_2", "hello [[world test 21344]]\n",
array( 'LinksUpdate' => array( 'mRecursive' => true,
'mLinks' => array( array( 'World_test_21344' => 0 ) ) ) )
),
// @todo: more...?
);
}
/**
* @dataProvider dataGetSecondaryDataUpdates
*/
public function testGetSecondaryDataUpdates( $title, $text, $expectedStuff ) {
$title = Title::newFromText( $title );
$title->resetArticleID( 2342 ); //dummy id. fine as long as we don't try to execute the updates!
$handler = ContentHandler::getForModelID( $title->getContentModel() );
$content = ContentHandler::makeContent( $text, $title );
$updates = $handler->getSecondaryDataUpdates( $content, $title );
// make updates accessible by class name
foreach ( $updates as $update ) {
$class = get_class( $update );
$updates[ $class ] = $update;
}
foreach ( $expectedStuff as $class => $fieldValues ) {
$this->assertArrayHasKey( $class, $updates, "missing an update of type $class" );
$update = $updates[ $class ];
foreach ( $fieldValues as $field => $value ) {
$v = $update->$field; #if the field doesn't exist, just crash and burn
$this->assertEquals( $value, $v, "unexpected value for field $field in instance of $class" );
}
}
}
public function dataGetDeletionUpdates() {
return array(
array("ContentHandlerTest_testGetSecondaryDataUpdates_1", "hello ''world''\n",
array( 'LinksDeletionUpdate' => array( ) )
),
array("ContentHandlerTest_testGetSecondaryDataUpdates_2", "hello [[world test 21344]]\n",
array( 'LinksDeletionUpdate' => array( ) )
),
// @todo: more...?
);
}
/**
* @dataProvider dataGetDeletionUpdates
*/
public function testDeletionUpdates( $title, $text, $expectedStuff ) {
$title = Title::newFromText( $title );
$title->resetArticleID( 2342 ); //dummy id. fine as long as we don't try to execute the updates!
$handler = ContentHandler::getForModelID( $title->getContentModel() );
$content = ContentHandler::makeContent( $text, $title );
$updates = $handler->getDeletionUpdates( $content, $title );
// make updates accessible by class name
foreach ( $updates as $update ) {
$class = get_class( $update );
$updates[ $class ] = $update;
}
foreach ( $expectedStuff as $class => $fieldValues ) {
$this->assertArrayHasKey( $class, $updates, "missing an update of type $class" );
$update = $updates[ $class ];
foreach ( $fieldValues as $field => $value ) {
$v = $update->$field; #if the field doesn't exist, just crash and burn
$this->assertEquals( $value, $v, "unexpected value for field $field in instance of $class" );
}
}
}
}
class DummyContentHandlerForTesting extends ContentHandler {