r96516 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r96515‎ | r96516 | r96517 >
Date:22:57, 7 September 2011
Author:nelson
Status:reverted (Comments)
Tags:
Comment:
split out the inside of File::transform() to avoid copying all of transform for SwiftMedia
Modified paths:
  • /trunk/phase3/includes/filerepo/File.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/filerepo/File.php
@@ -668,6 +668,51 @@
669669 }
670670
671671 /**
 672+ * Do the work of a transform (from an original into a thumb).
 673+ * Contains filesystem-specific functions.
 674+ *
 675+ * @param $thumbName string: the name of the thumbnail file.
 676+ * @param $thumbUrl string: the URL of the thumbnail file.
 677+ * @param $params Array: an associative array of handler-specific parameters.
 678+ * Typical keys are width, height and page.
 679+ *
 680+ * @return MediaTransformOutput | false
 681+ */
 682+ protected function maybeDoTransform( $thumbName, $thumbUrl, $params ) {
 683+ global $wgIgnoreImageErrors, $wgThumbnailEpoch;
 684+
 685+ $thumbPath = $this->getThumbPath( $thumbName );
 686+ if ( $this->repo && $this->repo->canTransformVia404() && !($flags & self::RENDER_NOW ) ) {
 687+ return $this->handler->getTransform( $this, $thumbPath, $thumbUrl, $params );
 688+ }
 689+
 690+ wfDebug( __METHOD__.": Doing stat for $thumbPath\n" );
 691+ $this->migrateThumbFile( $thumbName );
 692+ if ( file_exists( $thumbPath )) {
 693+ $thumbTime = filemtime( $thumbPath );
 694+ if ( $thumbTime !== FALSE &&
 695+ gmdate( 'YmdHis', $thumbTime ) >= $wgThumbnailEpoch ) {
 696+
 697+ return $this->handler->getTransform( $this, $thumbPath, $thumbUrl, $params );
 698+ }
 699+ }
 700+ $thumb = $this->handler->doTransform( $this, $thumbPath, $thumbUrl, $params );
 701+
 702+ // Ignore errors if requested
 703+ if ( !$thumb ) {
 704+ $thumb = null;
 705+ } elseif ( $thumb->isError() ) {
 706+ $this->lastError = $thumb->toText();
 707+ if ( $wgIgnoreImageErrors && !($flags & self::RENDER_NOW) ) {
 708+ $thumb = $this->handler->getTransform( $this, $thumbPath, $thumbUrl, $params );
 709+ }
 710+ }
 711+
 712+ return $thumb;
 713+ }
 714+
 715+
 716+ /**
672717 * Transform a media file
673718 *
674719 * @param $params Array: an associative array of handler-specific parameters.
@@ -676,7 +721,7 @@
677722 * @return MediaTransformOutput | false
678723 */
679724 function transform( $params, $flags = 0 ) {
680 - global $wgUseSquid, $wgIgnoreImageErrors, $wgThumbnailEpoch, $wgServer;
 725+ global $wgUseSquid, $wgServer;
681726
682727 wfProfileIn( __METHOD__ );
683728 do {
@@ -704,37 +749,10 @@
705750 $normalisedParams = $params;
706751 $this->handler->normaliseParams( $this, $normalisedParams );
707752 $thumbName = $this->thumbName( $normalisedParams );
708 - $thumbPath = $this->getThumbPath( $thumbName );
709753 $thumbUrl = $this->getThumbUrl( $thumbName );
710754
711 - if ( $this->repo && $this->repo->canTransformVia404() && !($flags & self::RENDER_NOW ) ) {
712 - $thumb = $this->handler->getTransform( $this, $thumbPath, $thumbUrl, $params );
713 - break;
714 - }
 755+ $thumb = $this->maybeDoTransform( $thumbName, $thumbUrl, $params );
715756
716 - wfDebug( __METHOD__.": Doing stat for $thumbPath\n" );
717 - $this->migrateThumbFile( $thumbName );
718 - if ( file_exists( $thumbPath )) {
719 - $thumbTime = filemtime( $thumbPath );
720 - if ( $thumbTime !== FALSE &&
721 - gmdate( 'YmdHis', $thumbTime ) >= $wgThumbnailEpoch ) {
722 -
723 - $thumb = $this->handler->getTransform( $this, $thumbPath, $thumbUrl, $params );
724 - break;
725 - }
726 - }
727 - $thumb = $this->handler->doTransform( $this, $thumbPath, $thumbUrl, $params );
728 -
729 - // Ignore errors if requested
730 - if ( !$thumb ) {
731 - $thumb = null;
732 - } elseif ( $thumb->isError() ) {
733 - $this->lastError = $thumb->toText();
734 - if ( $wgIgnoreImageErrors && !($flags & self::RENDER_NOW) ) {
735 - $thumb = $this->handler->getTransform( $this, $thumbPath, $thumbUrl, $params );
736 - }
737 - }
738 -
739757 // Purge. Useful in the event of Core -> Squid connection failure or squid
740758 // purge collisions from elsewhere during failure. Don't keep triggering for
741759 // "thumbs" which have the main image URL though (bug 13776)

Follow-up revisions

RevisionCommit summaryAuthorDate
r96599Fix for r96516: undefined variable $flagsialex19:11, 8 September 2011
r110641Reverted r96516, manually, per CR. Moved maybeDoTransform() code back into tr...aaron06:16, 3 February 2012

Comments

#Comment by Aaron Schulz (talk | contribs)   08:44, 8 September 2011

$flags is not defined in maybeDoTransform.

#Comment by IAlex (talk | contribs)   19:12, 8 September 2011

Fixed in r96599.

#Comment by Tim Starling (talk | contribs)   05:02, 3 February 2012

This causes the Squid purge to be done unconditionally, instead of only when the thumbnail is actually generated. So every time a page is rendered, every thumbnail on it is purged. I suggest a revert, since the more recent swift support presumably doesn't need this.

Status & tagging log