r98710 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r98709‎ | r98710 | r98711 >
Date:20:08, 2 October 2011
Author:btongminh
Status:reverted (Comments)
Tags:
Comment:
Follow-up r84395: Give MediaHandlers the option to remove items from the thumbnail purge list. Is needed for TMH because thumbnail rendering may be very expensive for media files. Haven't thought yet of a way to integrate this with the UI, but the current framework should be flexible enough to implement that later.

Added array $options to File::purgeCache(), LocalFile::purgeCache(), LocalFile::purgeThumbnails(), ForeignAPIFile::purgeCache() and ForeignAPIFile::purgeThumbnails() which is currently empty, but can be used later to indicate a full or partial purge.
Added MediaHandler::filterThumbnailPurgeList(), which can remove items from the purge list and also gets passed this $options array
Modified paths:
  • /trunk/phase3/includes/filerepo/File.php (modified) (history)
  • /trunk/phase3/includes/filerepo/ForeignAPIFile.php (modified) (history)
  • /trunk/phase3/includes/filerepo/LocalFile.php (modified) (history)
  • /trunk/phase3/includes/media/Generic.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/filerepo/ForeignAPIFile.php
——@@ -209,8 +209,11 @@
210210 return $files;
211211 }
212212
213 - function purgeCache() {
214 - $this->purgeThumbnails();
 213+ /**
 214+ * @see File::purgeCache()
 215+ */
 216+ function purgeCache( $options = array() ) {
 217+ $this->purgeThumbnails( $options );
215218 $this->purgeDescriptionPage();
216219 }
217220
——@@ -221,11 +224,18 @@
222225 $wgMemc->delete( $key );
223226 }
224227
225 - function purgeThumbnails() {
 228+ function purgeThumbnails( $options = array() ) {
226229 global $wgMemc;
227230 $key = $this->repo->getLocalCacheKey( 'ForeignAPIRepo', 'ThumbUrl', $this->getName() );
228231 $wgMemc->delete( $key );
 232+
229233 $files = $this->getThumbnails();
 234+ // Give media handler a chance to filter the purge list
 235+ $handler = $this->getHandler();
 236+ if ( $handler ) {
 237+ $handler->filterThumbnailPurgeList( $files, $options );
 238+ }
 239+
230240 $dir = $this->getThumbPath( $this->getName() );
231241 foreach ( $files as $file ) {
232242 unlink( $dir . $file );
Index: trunk/phase3/includes/filerepo/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() ) );
——@@ -720,10 +720,18 @@
721721 /**
722722 * Delete cached transformed files for the current version only.
723723 */
724 - function purgeThumbnails() {
 724+ function purgeThumbnails( $options = array() ) {
725725 global $wgUseSquid;
726 - // get a list of thumbnails and URLs
 726+
 727+ // Get a list of thumbnails and URLs
727728 $files = $this->getThumbnails();
 729+
 730+ // Give media handler a chance to filter the purge list
 731+ $handler = $this->getHandler();
 732+ if ( $handler ) {
 733+ $handler->filterThumbnailPurgeList( $files, $options );
 734+ }
 735+
728736 $dir = array_shift( $files );
729737 $this->purgeThumbList( $dir, $files );
730738
Index: trunk/phase3/includes/filerepo/File.php
——@@ -823,8 +823,9 @@
824824 * Purge shared caches such as thumbnails and DB data caching
825825 * STUB
826826 * Overridden by LocalFile
 827+ * @param array $options Array with options, currently undefined
827828 */
828 - function purgeCache() {}
 829+ function purgeCache( $options = array() ) {}
829830
830831 /**
831832 * Purge the file description page, but don't go after
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
r101088Reverted r84395,r98710: thumbnails must be purged on file deletionaaron23:34, 27 October 2011
r104410Restored r98710 but with a 'forRefresh' option (not used yet)aaron08:53, 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
r84395(bug 27641) purgeThumbnails should support exclusion of expensive files. Add ...catrope16:38, 20 March 2011

Comments

#Comment by Mdale (talk | contribs)   23:58, 14 November 2011

I see this was reverted? Why would the above code result in thumbnails not being purged on deletion?

What is the current solution for avoiding the removal of expensive to generate thumbnails ( ie video transcodes ) on simple action=purge page requests ?

#Comment by Aaron Schulz (talk | contribs)   01:29, 22 November 2011

This should probably be restored. I reverted it as it was a follow-up to another revision I was reverting. Also, conflicts occurred trying to just revert that one.

That said, they don't seem to be very related, so this should be restored.

Status & tagging log