r68587 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r68586‎ | r68587 | r68588 >
Date:21:03, 25 June 2010
Author:werdna
Status:resolved (Comments)
Tags:
Comment:
Some import-related fixes for LiquidThreads dump importation.
Modified paths:
  • /trunk/phase3/includes/ImportXMLReader.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialImport.php (modified) (history)
  • /trunk/phase3/maintenance/importDump.php (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/importDump.php
——@@ -150,6 +150,10 @@
151151 array( &$this, 'handleUpload' ) );
152152 $this->logItemCallback = $importer->setLogItemCallback(
153153 array( &$this, 'handleLogItem' ) );
 154+
 155+ if ( $this->dryRun ) {
 156+ $importer->setPageOutCallback( null );
 157+ }
154158
155159 return $importer->doImport();
156160 }
Index: trunk/phase3/includes/ImportXMLReader.php
——@@ -13,7 +13,7 @@
1414 * Creates an ImportXMLReader drawing from the source provided
1515 */
1616 function __construct( $source ) {
17 - $this->reader = new XMLReader();
 17+ $this->reader = new XMLReader2();
1818
1919 stream_wrapper_register( 'uploadsource', 'UploadSourceAdapter' );
2020 $id = UploadSourceAdapter::registerSource( $source );
——@@ -23,6 +23,7 @@
2424 $this->setRevisionCallback( array( $this, "importRevision" ) );
2525 $this->setUploadCallback( array( $this, 'importUpload' ) );
2626 $this->setLogItemCallback( array( $this, 'importLogItem' ) );
 27+ $this->setPageOutCallback( array( $this, 'finishImportPage' ) );
2728 }
2829
2930 private function throwXmlError( $err ) {
——@@ -168,6 +169,13 @@
169170 //return $dbw->deadlockLoop( array( $revision, 'importUpload' ) );
170171 return false;
171172 }
 173+
 174+ /**
 175+ * Mostly for hook use
 176+ */
 177+ public function finishImportPage( $title, $origTitle, $revCount, $sRevCount, $pageInfo ) {
 178+ return wfRunHooks( 'AfterImportPage', func_get_args() );
 179+ }
172180
173181 /**
174182 * Alternate per-revision callback, for debugging.
——@@ -203,10 +211,9 @@
204212 * @param $revisionCount int
205213 * @param $successCount Int: number of revisions for which callback returned true
206214 */
207 - private function pageOutCallback( $title, $origTitle, $revisionCount, $successCount ) {
 215+ private function pageOutCallback( $title, $origTitle, $revCount, $sucCount, $pageInfo ) {
208216 if( isset( $this->mPageOutCallback ) ) {
209 - call_user_func_array( $this->mPageOutCallback,
210 - array( $title, $origTitle, $revisionCount, $successCount ) );
 217+ call_user_func_array( $this->mPageOutCallback, func_get_args() );
211218 }
212219 }
213220
——@@ -244,21 +251,7 @@
245252 * @access private
246253 */
247254 private function nodeContents() {
248 - if( $this->reader->isEmptyElement ) {
249 - return "";
250 - }
251 - $buffer = "";
252 - while( $this->reader->read() ) {
253 - switch( $this->reader->nodeType ) {
254 - case XmlReader::TEXT:
255 - case XmlReader::SIGNIFICANT_WHITESPACE:
256 - $buffer .= $this->reader->value;
257 - break;
258 - case XmlReader::END_ELEMENT:
259 - return $buffer;
260 - }
261 - }
262 - return $this->close();
 255+ return $this->reader->nodeContents();
263256 }
264257
265258 # --------------
——@@ -378,7 +371,7 @@
379372 if ( !wfRunHooks( 'ImportHandleLogItemXMLTag',
380373 $this->reader, $logInfo ) ) {
381374 // Do nothing
382 - } if ( in_array( $tag, $normalFields ) ) {
 375+ } elseif ( in_array( $tag, $normalFields ) ) {
383376 $logInfo[$tag] = $this->nodeContents();
384377 } elseif ( $tag == 'contributor' ) {
385378 $logInfo['contributor'] = $this->handleContributor();
——@@ -436,10 +429,10 @@
437430 if ( $badTitle ) {
438431 // The title is invalid, bail out of this page
439432 $skip = true;
440 - } elseif ( !wfRunHooks( 'ImportHandlePageXMLTag', $this->reader,
441 - $pageInfo ) ) {
 433+ } elseif ( !wfRunHooks( 'ImportHandlePageXMLTag', array( $this->reader,
 434+ &$pageInfo ) ) ) {
442435 // Do nothing
443 - } if ( in_array( $tag, $normalFields ) ) {
 436+ } elseif ( in_array( $tag, $normalFields ) ) {
444437 $pageInfo[$tag] = $this->nodeContents();
445438 if ( $tag == 'title' ) {
446439 $title = $this->processTitle( $pageInfo['title'] );
——@@ -464,7 +457,8 @@
465458
466459 $this->pageOutCallback( $pageInfo['_title'], $origTitle,
467460 $pageInfo['revisionCount'],
468 - $pageInfo['successfulRevisionCount'] );
 461+ $pageInfo['successfulRevisionCount'],
 462+ $pageInfo );
469463 }
470464
471465 private function handleRevision( &$pageInfo ) {
——@@ -486,7 +480,7 @@
487481 if ( !wfRunHooks( 'ImportHandleRevisionXMLTag', $this->reader,
488482 $pageInfo, $revisionInfo ) ) {
489483 // Do nothing
490 - } if ( in_array( $tag, $normalFields ) ) {
 484+ } elseif ( in_array( $tag, $normalFields ) ) {
491485 $revisionInfo[$tag] = $this->nodeContents();
492486 } elseif ( $tag == 'contributor' ) {
493487 $revisionInfo['contributor'] = $this->handleContributor();
——@@ -547,7 +541,7 @@
548542 if ( !wfRunHooks( 'ImportHandleUploadXMLTag', $this->reader,
549543 $pageInfo, $revisionInfo ) ) {
550544 // Do nothing
551 - } if ( in_array( $tag, $normalFields ) ) {
 545+ } elseif ( in_array( $tag, $normalFields ) ) {
552546 $uploadInfo[$tag] = $this->nodeContents();
553547 } elseif ( $tag == 'contributor' ) {
554548 $uploadInfo['contributor'] = $this->handleContributor();
——@@ -712,3 +706,23 @@
713707 return $result;
714708 }
715709 }
 710+
 711+class XMLReader2 extends XMLReader {
 712+ function nodeContents() {
 713+ if( $this->isEmptyElement ) {
 714+ return "";
 715+ }
 716+ $buffer = "";
 717+ while( $this->read() ) {
 718+ switch( $this->nodeType ) {
 719+ case XmlReader::TEXT:
 720+ case XmlReader::SIGNIFICANT_WHITESPACE:
 721+ $buffer .= $this->value;
 722+ break;
 723+ case XmlReader::END_ELEMENT:
 724+ return $buffer;
 725+ }
 726+ }
 727+ return $this->close();
 728+ }
 729+}
Index: trunk/phase3/includes/specials/SpecialImport.php
——@@ -275,10 +275,12 @@
276276 class ImportReporter {
277277 private $reason=false;
278278 private $mOriginalLogCallback = null;
 279+ private $mOriginalPageOutCallback = null;
279280 private $mLogItemCount = 0;
280281
281282 function __construct( $importer, $upload, $interwiki , $reason=false ) {
282 - $importer->setPageOutCallback( array( $this, 'reportPage' ) );
 283+ $this->mOriginalPageOutCallback =
 284+ $importer->setPageOutCallback( array( $this, 'reportPage' ) );
283285 $this->mOriginalLogCallback =
284286 $importer->setLogItemCallback( array( $this, 'reportLogItem' ) );
285287 $this->mPageCount = 0;
——@@ -301,6 +303,8 @@
302304
303305 function reportPage( $title, $origTitle, $revisionCount, $successCount ) {
304306 global $wgOut, $wgUser, $wgLang, $wgContLang;
 307+
 308+ call_user_func_array( $this->mOriginalPageOutCallback, func_get_args() );
305309
306310 $skin = $wgUser->getSkin();
307311

Follow-up revisions

RevisionCommit summaryAuthorDate
r81437Kill the XMLReader2 class in response to Tim's code review comments on r68587werdna01:25, 3 February 2011

Comments

#Comment by Reedy (talk | contribs)   21:27, 12 January 2011

XMLReader2 really needs documenting to say what's different in it to the original...

#Comment by Tim Starling (talk | contribs)   06:36, 31 January 2011

I think XMLReader2 is a confusing name which doesn't explain what it does, and it's likely to conflict with future versions of the built-in class. Deriving a userspace class from a built-in class seems like a fragile, scary thing to do, and it should only be done if there's some reason for it. I liked the old code better. If you passed $this to the ImportHandlePageXMLTag hook instead of $this->reader, as you should have been doing anyway, then you wouldn't need this hack.

LiquidThreads is the only extension in Subversion that uses ImportHandlePageXMLTag, so I don't think it'll be too bad to break backwards compatibility. ImportHandleLogItemXMLTag, ImportHandleRevisionXMLTag and ImportHandleUploadXMLTag should be changed too, for consistency.

#Comment by Tim Starling (talk | contribs)   07:55, 31 January 2011

By the way, was XMLReader::readString() the function you were looking for? It seems to do roughly the same thing as your nodeContents(), except it doesn't return "1" as the contents of the node if the end of the file is reached.

Status & tagging log