r104410 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r104409‎ | r104410 | r104411 >
Date:08:53, 28 November 2011
Author:aaron
Status:ok (Comments)
Tags:
Comment:
Restored r98710 but with a 'forRefresh' option (not used yet)
Modified paths:
  • /trunk/phase3/includes/WikiFilePage.php (modified) (history)
  • /trunk/phase3/includes/filerepo/file/File.php (modified) (history)
  • /trunk/phase3/includes/filerepo/file/ForeignAPIFile.php (modified) (history)
  • /trunk/phase3/includes/filerepo/file/LocalFile.php (modified) (history)
  • /trunk/phase3/includes/media/Generic.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/WikiFilePage.php
@@ -156,12 +156,12 @@
157157 $update = new HTMLCacheUpdate( $this->mTitle, 'imagelinks' );
158158 $update->doUpdate();
159159 $this->mFile->upgradeRow();
160 - $this->mFile->purgeCache();
 160+ $this->mFile->purgeCache( array( 'forRefresh' => true ) );
161161 } else {
162162 wfDebug( 'ImagePage::doPurge no image for ' . $this->mFile->getName() . "; limiting purge to cache only\n" );
163163 // even if the file supposedly doesn't exist, force any cached information
164164 // to be updated (in case the cached information is wrong)
165 - $this->mFile->purgeCache();
 165+ $this->mFile->purgeCache( array( 'forRefresh' => true ) );
166166 }
167167 parent::doPurge();
168168 }
Index: trunk/phase3/includes/filerepo/file/LocalFile.php
@@ -675,12 +675,12 @@
676676 /**
677677 * Delete all previously generated thumbnails, refresh metadata in memcached and purge the squid
678678 */
679 - function purgeCache() {
 679+ function purgeCache( $options = array() ) {
680680 // Refresh metadata cache
681681 $this->purgeMetadataCache();
682682
683683 // Delete thumbnails
684 - $this->purgeThumbnails();
 684+ $this->purgeThumbnails( $options );
685685
686686 // Purge squid cache for this file
687687 SquidUpdate::purge( array( $this->getURL() ) );
@@ -723,11 +723,18 @@
724724 /**
725725 * Delete cached transformed files for the current version only.
726726 */
727 - function purgeThumbnails() {
 727+ function purgeThumbnails( $options ) {
728728 global $wgUseSquid;
729729
730730 // Delete thumbnails
731731 $files = $this->getThumbnails();
 732+
 733+ // Give media handler a chance to filter the purge list
 734+ $handler = $this->getHandler();
 735+ if ( $handler ) {
 736+ $handler->filterThumbnailPurgeList( $files, $options );
 737+ }
 738+
732739 $dir = array_shift( $files );
733740 $this->purgeThumbList( $dir, $files );
734741
Index: trunk/phase3/includes/filerepo/file/File.php
@@ -863,8 +863,10 @@
864864 * Purge shared caches such as thumbnails and DB data caching
865865 * STUB
866866 * Overridden by LocalFile
 867+ * @param $options Array Options, which include:
 868+ * 'forRefresh' : The purging is only to refresh thumbnails
867869 */
868 - function purgeCache() {}
 870+ function purgeCache( $options = array() ) {}
869871
870872 /**
871873 * Purge the file description page, but don't go after
Index: trunk/phase3/includes/filerepo/file/ForeignAPIFile.php
@@ -214,8 +214,11 @@
215215 return $files;
216216 }
217217
218 - function purgeCache() {
219 - $this->purgeThumbnails();
 218+ /**
 219+ * @see File::purgeCache()
 220+ */
 221+ function purgeCache( $options = array() ) {
 222+ $this->purgeThumbnails( $options );
220223 $this->purgeDescriptionPage();
221224 }
222225
@@ -226,11 +229,17 @@
227230 $wgMemc->delete( $key );
228231 }
229232
230 - function purgeThumbnails() {
 233+ function purgeThumbnails( $options = array() ) {
231234 global $wgMemc;
232235 $key = $this->repo->getLocalCacheKey( 'ForeignAPIRepo', 'ThumbUrl', $this->getName() );
233236 $wgMemc->delete( $key );
234237 $files = $this->getThumbnails();
 238+ // Give media handler a chance to filter the purge list
 239+ $handler = $this->getHandler();
 240+ if ( $handler ) {
 241+ $handler->filterThumbnailPurgeList( $files, $options );
 242+ }
 243+
235244 $dir = $this->getThumbPath( $this->getName() );
236245 foreach ( $files as $file ) {
237246 unlink( $dir . $file );
Index: trunk/phase3/includes/media/Generic.php
@@ -504,6 +504,16 @@
505505 }
506506 return false;
507507 }
 508+
 509+ /**
 510+ * Remove files from the purge list
 511+ *
 512+ * @param array $files
 513+ * @param array $options
 514+ */
 515+ public function filterThumbnailPurgeList( &$files, $options ) {
 516+ // Do nothing
 517+ }
508518 }
509519
510520 /**

Follow-up revisions

RevisionCommit summaryAuthorDate
r104411FU r104410: don't even bother calling filterThumbnailPurgeList() if 'forRefre...aaron08:57, 28 November 2011
r104412Fix r104410: Added default value for $options to purgeThumbnails(), not sure ...aaron09:03, 28 November 2011
r106761follow-up r98710/r104410. I personally think that forRefresh is confusing, si...bawolff06:48, 20 December 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r98710Follow-up r84395: Give MediaHandlers the option to remove items from the thum...btongminh20:08, 2 October 2011

Comments

#Comment by Bawolff (talk | contribs)   05:38, 20 December 2011

I don't really like the name 'forRefresh' - its kind of confusing as it could be referring to refreshing the page, or something else. Perhaps forThumbRefresh would be more clear.

#Comment by Aaron Schulz (talk | contribs)   06:18, 20 December 2011

{{sofixit}}

Status & tagging log