r68699 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r68698‎ | r68699 | r68700 >
Date:02:57, 29 June 2010
Author:aaron
Status:ok
Tags:
Comment:
* Use includesAreSynced() instead of fr_newestTemplateId stuff
* Added sanity checks to includesAreSynced()
* Removed fr_newestTemplateId/fr_newestImageTime (no longer used)
* Use parserInjectTimestamps() for stable output too, so deleted files handle properly
* Cleaned-up expandText() (unused atm)
Modified paths:
  • /trunk/extensions/FlaggedRevs/FlaggedArticleView.php (modified) (history)
  • /trunk/extensions/FlaggedRevs/FlaggedRevs.class.php (modified) (history)
  • /trunk/extensions/FlaggedRevs/FlaggedRevs.hooks.php (modified) (history)
  • /trunk/extensions/FlaggedRevs/forms/RevisionReviewForm.php (modified) (history)

Diff [purge]

Index: trunk/extensions/FlaggedRevs/FlaggedRevs.class.php
——@@ -462,24 +462,23 @@
463463 * @param string $text
464464 * @param Title $title
465465 * @param integer $id, revision id
466 - * @return array( string, array, array, array, int )
 466+ * @return array( string, array, array )
467467 * All included pages/arguments are expanded out
468468 */
469469 public static function expandText( $text = '', Title $title, $id ) {
470470 global $wgParser;
471471 # Make our hooks trigger (force unstub so setting doesn't get lost)
472472 $wgParser->firstCallInit();
 473+ # Begin stable parse
473474 $wgParser->fr_isStable = ( self::inclusionSetting() != FR_INCLUDES_CURRENT );
474475 # Parse with default options
475476 $options = self::makeParserOptions();
476477 $outputText = $wgParser->preprocess( $text, $title, $options, $id );
477478 $out =& $wgParser->mOutput;
478 - $data = array( $outputText, $out->mTemplates, $out->mTemplateIds,
479 - $out->fr_includeErrors, $out->fr_newestTemplateID );
480 - # Done!
 479+ # Stable parse done!
481480 $wgParser->fr_isStable = false;
482481 # Return data array
483 - return $data;
 482+ return array( $outputText, $out->mTemplateIds, $out->fr_includeErrors );
484483 }
485484
486485 /**
——@@ -686,11 +685,12 @@
687686 }
688687 return null; // cache not found
689688 }
690 -
 689+
691690 # ################ Synchronization and link update functions #################
692691
693692 /**
694 - * Check if all includes in $stableOutput are the same as those in $currentOutput
 693+ * Check if all includes in $stableOutput are the same as those in $currentOutput.
 694+ * Only use this if both outputs are from the same source text.
695695 * @param ParserOutput $stableOutput
696696 * @param ParserOutput $currentOutput
697697 * @return bool
——@@ -698,19 +698,27 @@
699699 public static function includesAreSynced(
700700 ParserOutput $stableOutput, ParserOutput $currentOutput
701701 ) {
702 - $sTmpls = $stableOutput->mTemplateIds;
703 - $cTmpls = $currentOutput->mTemplateIds;
704 - foreach ( $sTmpls as $name => $revId ) {
705 - if ( isset( $cTmpls[$name] ) && $cTmpls[$name] != $revId ) {
706 - return false; // updated/created
 702+ if ( isset( $stableOutput->mTemplateIds ) // sanity check
 703+ && isset( $currentOutput->mTemplateIds ) )
 704+ {
 705+ $sTmpls = $stableOutput->mTemplateIds;
 706+ $cTmpls = $currentOutput->mTemplateIds;
 707+ foreach ( $sTmpls as $name => $revId ) {
 708+ if ( isset( $cTmpls[$name] ) && $cTmpls[$name] != $revId ) {
 709+ return false; // updated/created
 710+ }
707711 }
708712 }
709 - $sFiles = $stableOutput->fr_ImageSHA1Keys;
710 - $cFiles = $currentOutput->fr_ImageSHA1Keys;
711 - foreach ( $sFiles as $name => $timeKey ) {
712 - foreach ( $timeKey as $sTs => $sSha1 ) {
713 - if ( isset( $cFiles[$name] ) && !isset( $cFiles[$name][$sTs] ) ) {
714 - return false; // updated/created
 713+ if ( isset( $stableOutput->fr_ImageSHA1Keys ) // sanity check
 714+ && isset( $currentOutput->fr_ImageSHA1Keys ) )
 715+ {
 716+ $sFiles = $stableOutput->fr_ImageSHA1Keys;
 717+ $cFiles = $currentOutput->fr_ImageSHA1Keys;
 718+ foreach ( $sFiles as $name => $timeKey ) {
 719+ foreach ( $timeKey as $sTs => $sSha1 ) {
 720+ if ( isset( $cFiles[$name] ) && !isset( $cFiles[$name][$sTs] ) ) {
 721+ return false; // updated/created
 722+ }
715723 }
716724 }
717725 }
Index: trunk/extensions/FlaggedRevs/forms/RevisionReviewForm.php
——@@ -482,8 +482,8 @@
483483 $parserCache = ParserCache::singleton();
484484 $poutput = $parserCache->get( $article, $this->user );
485485 if ( !$poutput
486 - || !isset( $poutput->fr_newestTemplateID )
487 - || !isset( $poutput->fr_newestImageTime ) )
 486+ || !isset( $poutput->fr_ImageSHA1Keys )
 487+ || !isset( $poutput->mTemplateIds ) )
488488 {
489489 $source = $article->getContent();
490490 $options = FlaggedRevs::makeParserOptions();
——@@ -503,19 +503,15 @@
504504 if ( !Title::newFromRedirect( $text ) ) {
505505 FlaggedRevs::updatePageCache( $article, $this->user, $stableOutput );
506506 }
507 - # We can set the sync cache key already
508 - $includesSynced = true;
509 - if ( $poutput->fr_newestImageTime > $stableOutput->fr_newestImageTime ) {
510 - $includesSynced = false;
511 - } elseif ( $poutput->fr_newestTemplateID > $stableOutput->fr_newestTemplateID ) {
512 - $includesSynced = false;
513 - }
514507 $u->fr_stableRev = $sv; // no need to re-fetch this!
515508 $u->fr_stableParserOut = $stableOutput; // no need to re-fetch this!
516 - # We can set the sync cache key already.
517 - $key = wfMemcKey( 'flaggedrevs', 'includesSynced', $article->getId() );
518 - $data = FlaggedRevs::makeMemcObj( $includesSynced ? "true" : "false" );
519 - $wgMemc->set( $key, $data, $wgParserCacheExpireTime );
 509+ # We can set the sync cache key already...
 510+ if ( $rev->isCurrent() ) {
 511+ $includesSynced = FlaggedRevs::includesAreSynced( $stableOutput, $poutput );
 512+ $key = wfMemcKey( 'flaggedrevs', 'includesSynced', $article->getId() );
 513+ $data = FlaggedRevs::makeMemcObj( $includesSynced ? "true" : "false" );
 514+ $wgMemc->set( $key, $data, $wgParserCacheExpireTime );
 515+ }
520516 } else {
521517 # Get the old stable cache
522518 $stableOutput = FlaggedRevs::getPageCache( $article, $this->user );
Index: trunk/extensions/FlaggedRevs/FlaggedRevs.hooks.php
——@@ -371,8 +371,6 @@
372372 */
373373 public static function parserAddFields( Parser $parser ) {
374374 $parser->mOutput->fr_ImageSHA1Keys = array();
375 - $parser->mOutput->fr_newestImageTime = "0";
376 - $parser->mOutput->fr_newestTemplateID = 0;
377375 $parser->mOutput->fr_includeErrors = array();
378376 return true;
379377 }
——@@ -433,9 +431,6 @@
434432 $skip = true; // If ID is zero, don't load it
435433 }
436434 }
437 - if ( $id > $parser->mOutput->fr_newestTemplateID ) {
438 - $parser->mOutput->fr_newestTemplateID = $id;
439 - }
440435 return true;
441436 }
442437
——@@ -450,7 +445,6 @@
451446 return true;
452447 }
453448 $file = null;
454 - $isKnownLocal = $isLocalFile = false; // local file on this wiki?
455449 # Normalize NS_MEDIA to NS_FILE
456450 if ( $nt->getNamespace() == NS_MEDIA ) {
457451 $title = Title::makeTitle( NS_FILE, $nt->getDBkey() );
——@@ -469,7 +463,6 @@
470464 if ( $srev && $srev->getFileTimestamp() ) {
471465 $time = $srev->getFileTimestamp(); // TS or null
472466 $sha1 = $srev->getFileSha1();
473 - $isKnownLocal = true; // must be local
474467 }
475468 }
476469 # Check cache before doing another DB hit...
——@@ -481,7 +474,6 @@
482475 if ( !$time || $timeP > $time ) {
483476 $time = $timeP;
484477 $sha1 = $sha1P;
485 - $isKnownLocal = false; // up in the air...possibly a commons image
486478 }
487479 }
488480 # If there is no stable version (or that feature is not enabled), use
——@@ -499,7 +491,6 @@
500492 if ( $row && ( $time === false || $reviewedTS > $time ) ) {
501493 $time = $reviewedTS;
502494 $sha1 = $row->fi_img_sha1;
503 - $isKnownLocal = false; // up in the air...possibly a commons image
504495 }
505496 }
506497 $query = $time ? "filetimestamp=" . urlencode( wfTimestamp( TS_MW, $time ) ) : "";
——@@ -523,20 +514,6 @@
524515 # Add image metadata to parser output
525516 $parser->mOutput->fr_ImageSHA1Keys[$title->getDBkey()] = array();
526517 $parser->mOutput->fr_ImageSHA1Keys[$title->getDBkey()][$time] = $sha1;
527 - # Check if this file is local or on a foreign repo...
528 - if ( $isKnownLocal ) {
529 - $isLocalFile = true; // no need to check
530 - # Load the image if needed (note that time === '0' means we have no image)
531 - } elseif ( $time !== "0" ) {
532 - # FIXME: would be nice not to double fetch!
533 - $file = $file ? $file : self::getLocalFile( $title, $time );
534 - $isLocalFile = $file && $file->exists() && $file->isLocal();
535 - }
536 - # Bug 15748, be lax about commons image sync status...
537 - # When getting the max timestamp, just ignore the commons image timestamps.
538 - if ( $isLocalFile && $time > $parser->mOutput->fr_newestImageTime ) {
539 - $parser->mOutput->fr_newestImageTime = $time;
540 - }
541518 return true;
542519 }
543520
——@@ -552,7 +529,6 @@
553530 return true; // nothing to do
554531 }
555532 $file = null;
556 - $isKnownLocal = $isLocalFile = false; // local file on this wiki?
557533 # Check for stable version of image if this feature is enabled.
558534 # Should be in reviewable namespace, this saves unneeded DB checks as
559535 # well as enforce site settings if they are later changed.
——@@ -564,7 +540,6 @@
565541 if ( $srev && $srev->getFileTimestamp() ) {
566542 $time = $srev->getFileTimestamp(); // TS or null
567543 $sha1 = $srev->getFileSha1();
568 - $isKnownLocal = true; // must be local
569544 }
570545 }
571546 # Check cache before doing another DB hit...
——@@ -575,7 +550,6 @@
576551 if ( !$time || $timeP > $time ) {
577552 $time = $timeP;
578553 $sha1 = $sha1P;
579 - $isKnownLocal = false; // up in the air...possibly a commons image
580554 }
581555 }
582556 # If there is no stable version (or that feature is not enabled), use
——@@ -592,7 +566,6 @@
593567 if ( $row && ( $time === false || $reviewedTS > $time ) ) {
594568 $time = $reviewedTS;
595569 $sha1 = $row->fi_img_sha1;
596 - $isKnownLocal = false; // up in the air...possibly a commons image
597570 }
598571 }
599572 $query = $time ? "filetimestamp=" . urlencode( wfTimestamp( TS_MW, $time ) ) : "";
——@@ -616,19 +589,6 @@
617590 # Add image metadata to parser output
618591 $parser->mOutput->fr_ImageSHA1Keys[$nt->getDBkey()] = array();
619592 $parser->mOutput->fr_ImageSHA1Keys[$nt->getDBkey()][$time] = $sha1;
620 - # Check if this file is local or on a foreign repo...
621 - if ( $isKnownLocal ) {
622 - $isLocalFile = true; // no need to check
623 - # Load the image if needed (note that time === '0' means we have no image)
624 - } elseif ( $time !== "0" ) {
625 - # FIXME: would be nice not to double fetch!
626 - $file = $file ? $file : self::getLocalFile( $nt, $time );
627 - $isLocalFile = $file && $file->exists() && $file->isLocal();
628 - }
629 - # Bug 15748, be lax about commons image sync status
630 - if ( $isLocalFile && $time > $parser->mOutput->fr_newestImageTime ) {
631 - $parser->mOutput->fr_newestImageTime = $time;
632 - }
633593 return true;
634594 }
635595
——@@ -636,43 +596,29 @@
637597 * Insert image timestamps/SHA-1 keys into parser output
638598 */
639599 public static function parserInjectTimestamps( Parser $parser ) {
640 - # Don't trigger this for stable version parsing...it will do it separately.
641 - if ( !empty( $parser->fr_isStable ) ) {
642 - return true;
 600+ $pOutput =& $parser->mOutput; // convenience
 601+ if ( !isset( $pOutput->mImages ) ) {
 602+ return true; // sanity check
643603 }
644 - $maxRevision = 0;
645 - # Record the max template revision ID
646 - if ( !empty( $parser->mOutput->mTemplateIds ) ) {
647 - foreach ( $parser->mOutput->mTemplateIds as $namespace => $DBKeyRev ) {
648 - foreach ( $DBKeyRev as $DBkey => $revID ) {
649 - if ( $revID > $maxRevision ) {
650 - $maxRevision = $revID;
651 - }
652 - }
653 - }
654 - }
655 - $parser->mOutput->fr_newestTemplateID = $maxRevision;
 604+ $stableOut = !empty( $parser->fr_isStable );
656605 # Fetch the current timestamps of the images.
657 - $maxTimestamp = "0";
658 - if ( !empty( $parser->mOutput->mImages ) ) {
659 - foreach ( $parser->mOutput->mImages as $filename => $x ) {
660 - # FIXME: it would be nice not to double fetch these!
661 - $file = wfFindFile( Title::makeTitleSafe( NS_FILE, $filename ) );
662 - $parser->mOutput->fr_ImageSHA1Keys[$filename] = array();
663 - if ( $file ) {
664 - $ts = $file->getTimestamp();
665 - # Bug 15748, be lax about commons image sync status
666 - if ( $file->isLocal() && $ts > $maxTimestamp ) {
667 - $maxTimestamp = $ts;
668 - }
669 - $parser->mOutput->fr_ImageSHA1Keys[$filename][$ts] = $file->getSha1();
670 - } else {
671 - $parser->mOutput->fr_ImageSHA1Keys[$filename]["0"] = '';
 606+ foreach ( $pOutput->mImages as $filename => $x ) {
 607+ # FIXME: it would be nice not to double fetch these!
 608+ $time = false; // current
 609+ if ( $stableOut && isset( $pOutput->fr_ImageSHA1Keys[$filename] ) ) {
 610+ foreach( $pOutput->fr_ImageSHA1Keys[$filename] as $time => $sha1 ) {
 611+ // Fetch file with $time to confirm the specified version exists
672612 }
673613 }
 614+ $title = Title::makeTitleSafe( NS_FILE, $filename );
 615+ $file = wfFindFile( $title, array( 'time' => $time ) );
 616+ $pOutput->fr_ImageSHA1Keys[$filename] = array();
 617+ if ( $file ) {
 618+ $pOutput->fr_ImageSHA1Keys[$filename][$file->getTimestamp()] = $file->getSha1();
 619+ } else {
 620+ $pOutput->fr_ImageSHA1Keys[$filename]["0"] = '';
 621+ }
674622 }
675 - # Record the max timestamp
676 - $parser->mOutput->fr_newestImageTime = $maxTimestamp;
677623 return true;
678624 }
679625
Index: trunk/extensions/FlaggedRevs/FlaggedArticleView.php
——@@ -1793,23 +1793,24 @@
17941794 public function maybeUpdateMainCache( &$outputDone, &$pcache ) {
17951795 global $wgUser, $wgRequest;
17961796 $this->load();
1797 -
17981797 $action = $wgRequest->getVal( 'action', 'view' );
1799 - if ( $action == 'purge' )
 1798+ if ( $action == 'purge' ) {
18001799 return true; // already purging!
 1800+ }
18011801 # Only trigger on article view for content pages, not for protect/delete/hist
1802 - if ( !self::isViewAction( $action ) || !$wgUser->isAllowed( 'review' ) )
 1802+ if ( !self::isViewAction( $action ) || !$wgUser->isAllowed( 'review' ) ) {
18031803 return true;
1804 - if ( !$this->article->exists() || !$this->article->isReviewable() )
 1804+ }
 1805+ if ( !$this->article->exists() || !$this->article->isReviewable() ) {
18051806 return true;
1806 -
 1807+ }
18071808 $parserCache = ParserCache::singleton();
18081809 $parserOut = $parserCache->get( $this->article, $wgUser );
18091810 if ( $parserOut ) {
18101811 # Clear older, incomplete, cached versions
18111812 # We need the IDs of templates and timestamps of images used
1812 - if ( !isset( $parserOut->fr_newestTemplateID )
1813 - || !isset( $parserOut->fr_newestImageTime ) )
 1813+ if ( !isset( $parserOut->fr_ImageSHA1Keys )
 1814+ || !isset( $parserOut->mTemplateIds ) )
18141815 {
18151816 $this->article->getTitle()->invalidateCache();
18161817 }

Status & tagging log