r96373 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r96372‎ \| r96373 \| r96374 >
Date:	21:01, 6 September 2011
Author:	demon
Status:	ok
Tags:
Comment:	(bug 30192) Thumbnails of archived images don't get deleted. Patch by Russ and Sam, with minor tweaks by me.
Modified paths:	/trunk/phase3/RELEASE-NOTES-1.19 (modified) (history) /trunk/phase3/includes/filerepo/File.php (modified) (history) /trunk/phase3/includes/filerepo/LocalFile.php (modified) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES-1.19
—	—	@@ -84,6 +84,7 @@
85	85	* (bug 30684) Fix bad escaping in mw.message for inexistent messages (i.e. <key>)
86	86	* (bug 23057) Importers no longer can 'edit' or 'create' a fully-protected page by
87	87	importing a new revision into it
	88	+* (bug 30192) Thumbnails of archived files are now deleted
88	89
89	90	=== API changes in 1.19 ===
90	91	* (bug 19838) siprop=interwikimap can now use the interwiki cache.
Index: trunk/phase3/includes/filerepo/LocalFile.php
—	—	@@ -613,12 +613,19 @@
614	614
615	615	/**
616	616	* Get all thumbnail names previously generated for this file
	617	+ * @param $archiveName string\|false Name of an archive file
	618	+ * @return array first element is the base dir, then files in that base dir.
617	619	*/
618		~~- function getThumbnails() {~~
	620	+ function getThumbnails( $archiveName = false ) {
619	621	$this->load();
620	622
	623	+ if ( $archiveName ) {
	624	+ $dir = $this->getArchiveThumbPath( $archiveName );
	625	+ } else {
	626	+ $dir = $this->getThumbPath();
	627	+ }
621	628	$files = array();
622		~~- $dir = $this->getThumbPath();~~
	629	+ $files[] = $dir;
623	630
624	631	if ( is_dir( $dir ) ) {
625	632	$handle = opendir( $dir );
—	—	@@ -680,16 +687,65 @@
681	688	}
682	689
683	690	/**
684		~~- * Delete cached transformed files~~
	691	+ * Delete cached transformed files for archived files
	692	+ * @param $archiveName string name of the archived file
685	693	*/
	694	+ function purgeOldThumbnails( $archiveName ) {
	695	+ global $wgUseSquid;
	696	+ // get a list of old thumbnails and URLs
	697	+ $files = $this->getThumbnails( $archiveName );
	698	+ $dir = array_shift( $files );
	699	+ $this->purgeThumbList( $dir, $files );
	700	+
	701	+ // Directory should be empty, delete it too. This will probably suck on
	702	+ // something like NFS or if the directory isn't actually empty, so hide
	703	+ // the warnings :D
	704	+ wfSuppressWarnings();
	705	+ if( !rmdir( $dir ) ) {
	706	+ wfDebug( __METHOD__ . ": unable to remove archive directory: $dir\n" );
	707	+ }
	708	+ wfRestoreWarnings();
	709	+
	710	+ // Purge the squid
	711	+ if ( $wgUseSquid ) {
	712	+ $urls = array();
	713	+ foreach( $files as $file ) {
	714	+ $urls[] = $this->getArchiveThumbUrl( $archiveName, $file );
	715	+ }
	716	+ SquidUpdate::purge( $urls );
	717	+ }
	718	+ }
	719	+
	720	+
	721	+ /**
	722	+ * Delete cached transformed files for the current version only.
	723	+ */
686	724	function purgeThumbnails() {
687		~~- global $wgUseSquid, $wgExcludeFromThumbnailPurge;~~
688		-
689		~~- // Delete thumbnails~~
	725	+ global $wgUseSquid;
	726	+ // get a list of thumbnails and URLs
690	727	$files = $this->getThumbnails();
691		~~- $dir = $this->getThumbPath();~~
692		~~- $urls = array();~~
	728	+ $dir = array_shift( $files );
	729	+ $this->purgeThumbList( $dir, $files );
693	730
	731	+ // Purge the squid
	732	+ if ( $wgUseSquid ) {
	733	+ $urls = array();
	734	+ foreach( $files as $file ) {
	735	+ $urls[] = $this->getThumbUrl( $file );
	736	+ }
	737	+ SquidUpdate::purge( $urls );
	738	+ }
	739	+ }
	740	+
	741	+ /**
	742	+ * Delete a list of thumbnails visible at urls
	743	+ * @param $dir string base dir of the files.
	744	+ * @param $files array of strings: relative filenames (to $dir)
	745	+ */
	746	+ function purgeThumbList($dir, $files) {
	747	+ global $wgExcludeFromThumbnailPurge;
	748	+
	749	+ wfDebug( __METHOD__ . ": " . var_export( $files, true ) . "\n" );
694	750	foreach ( $files as $file ) {
695	751	// Only remove files not in the $wgExcludeFromThumbnailPurge configuration variable
696	752	$ext = pathinfo( "$dir/$file", PATHINFO_EXTENSION );
—	—	@@ -700,18 +756,11 @@
701	757	# Check that the base file name is part of the thumb name
702	758	# This is a basic sanity check to avoid erasing unrelated directories
703	759	if ( strpos( $file, $this->getName() ) !== false ) {
704		~~- $url = $this->getThumbUrl( $file );~~
705		~~- $urls[] = $url;~~
706	760	wfSuppressWarnings();
707	761	unlink( "$dir/$file" );
708	762	wfRestoreWarnings();
709	763	}
710	764	}
711		-
712		~~- // Purge the squid~~
713		~~- if ( $wgUseSquid ) {~~
714		~~- SquidUpdate::purge( $urls );~~
715		~~- }~~
716	765	}
717	766
718	767	/** purgeDescription inherited */
—	—	@@ -1185,6 +1234,7 @@
1186	1235	array( 'oi_name' => $this->getName() ) );
1187	1236	foreach ( $result as $row ) {
1188	1237	$batch->addOld( $row->oi_archive_name );
	1238	+ $this->purgeOldThumbnails( $row->oi_archive_name );
1189	1239	}
1190	1240	$status = $batch->execute();
1191	1241
—	—	@@ -1219,6 +1269,7 @@
1220	1270
1221	1271	$batch = new LocalFileDeleteBatch( $this, $reason, $suppress );
1222	1272	$batch->addOld( $archiveName );
	1273	+ $this->purgeOldThumbnails( $archiveName );
1223	1274	$status = $batch->execute();
1224	1275
1225	1276	$this->unlock();
Index: trunk/phase3/includes/filerepo/File.php
—	—	@@ -890,14 +890,26 @@
891	891	}
892	892
893	893	/**
894		~~- * Get the relative path for an archive file~~
	894	+ * Get the relative path for an archived file
	895	+ *
	896	+ * @param $archiveName string the timestamped name of an archived image
	897	+ * @param $suffix bool\|string if not false, the name of a thumbnail file
895	898	*
896		~~- * @param $suffix bool~~
897		- *
898		~~- * @return string~~
	899	+ * @return string
899	900	*/
900		~~- function getArchiveRel( $suffix = false ) {~~
901		~~- $path = 'archive/' . $this->getHashPath();~~
	901	+ function getArchiveRel( $archiveName ) {
	902	+ return 'archive/' . $this->getHashPath() . $archiveName;
	903	+ }
	904	+
	905	+ /**
	906	+ * Get the relative path for an archived file's thumbs directory
	907	+ * or a specific thumb if the $suffix is given.
	908	+ *
	909	+ * @param $archiveName string the timestamped name of an archived image
	910	+ * @param $suffix bool\|string if not false, the name of a thumbnail file
	911	+ */
	912	+ function getArchiveThumbRel( $archiveName, $suffix = false ) {
	913	+ $path = 'archive/' . $this->getHashPath() . $archiveName . "/";
902	914	if ( $suffix === false ) {
903	915	$path = substr( $path, 0, -1 );
904	916	} else {
—	—	@@ -907,20 +919,32 @@
908	920	}
909	921
910	922	/**
911		~~- * Get the path of the archive directory, or a particular file if $suffix is specified~~
	923	+ * Get the path of the archived file.
912	924	*
913		~~- * @param $suffix bool~~
	925	+ * @param $archiveName the timestamped name of an archived image
914	926	*
915	927	* @return string
916	928	*/
917		~~- function getArchivePath( $suffix = false ) {~~
918		~~- return $this->repo->getZonePath( 'public' ) . '/' . $this->getArchiveRel( $suffix );~~
	929	+ function getArchivePath( $archiveName ) {
	930	+ return $this->repo->getZonePath( 'public' ) . '/' . $this->getArchiveRel( $archiveName );
919	931	}
920	932
921	933	/**
	934	+ * Get the path of the archived file's thumbs, or a particular thumb if $suffix is specified
	935	+ *
	936	+ * @param $archiveName string the timestamped name of an archived image
	937	+ * @param $suffix bool\|string if not false, the name of a thumbnail file
	938	+ *
	939	+ * @return string
	940	+ */
	941	+ function getArchiveThumbPath( $archiveName, $suffix = false ) {
	942	+ return $this->repo->getZonePath( 'thumb' ) . '/' . $this->getArchiveThumbRel( $archiveName, $suffix );
	943	+ }
	944	+
	945	+ /**
922	946	* Get the path of the thumbnail directory, or a particular file if $suffix is specified
923	947	*
924		~~- * @param $suffix bool~~
	948	+ * @param $suffix bool\|string if not false, the name of a thumbnail file
925	949	*
926	950	* @return string
927	951	*/
—	—	@@ -933,14 +957,26 @@
934	958	}
935	959
936	960	/**
937		~~- * Get the URL of the archive directory, or a particular file if $suffix is specified~~
	961	+ * Get the URL of the archived file
938	962	*
939		~~- * @param $suffix bool~~
	963	+ * @param $archiveName string
940	964	*
941	965	* @return string
942	966	*/
943		~~- function getArchiveUrl( $suffix = false ) {~~
944		~~- $path = $this->repo->getZoneUrl('public') . '/archive/' . $this->getHashPath();~~
	967	+ function getArchiveUrl( $archiveName ) {
	968	+ return $this->repo->getZoneUrl('public') . '/archive/' . $this->getHashPath() . rawurlencode( $archiveName );
	969	+ }
	970	+
	971	+ /**
	972	+ * Get the URL of the archived file's thumbs, or a particular thumb if $suffix is specified
	973	+ *
	974	+ * @param $archiveName string the timestamped name of an archived image
	975	+ * @param $suffix bool\|string if not false, the name of a thumbnail file
	976	+ *
	977	+ * @return string
	978	+ */
	979	+ function getArchiveThumbUrl( $archiveName, $suffix = false ) {
	980	+ $path = $this->repo->getZoneUrl('thumb') . '/archive/' . $this->getHashPath() . rawurlencode( $archiveName ) . "/";
945	981	if ( $suffix === false ) {
946	982	$path = substr( $path, 0, -1 );
947	983	} else {
—	—	@@ -952,7 +988,7 @@
953	989	/**
954	990	* Get the URL of the thumbnail directory, or a particular file if $suffix is specified
955	991	*
956		~~- * @param $suffix bool~~
	992	+ * @param $suffix bool\|string if not false, the name of a thumbnail file
957	993	*
958	994	* @return path
959	995	*/
—	—	@@ -965,9 +1001,9 @@
966	1002	}
967	1003
968	1004	/**
969		~~- * Get the virtual URL for an archive file or directory~~
	1005	+ * Get the virtual URL for an archived file's thumbs, or a specific thumb.
970	1006	*
971		~~- * @param bool\|string $suffix~~
	1007	+ * @param $suffix bool\|string if not false, the name of a thumbnail file
972	1008	*
973	1009	* @return string
974	1010	*/
—	—	@@ -984,7 +1020,7 @@
985	1021	/**
986	1022	* Get the virtual URL for a thumbnail file or directory
987	1023	*
988		~~- * @param $suffix bool~~
	1024	+ * @param $suffix bool\|string if not false, the name of a thumbnail file
989	1025	*
990	1026	* @return string
991	1027	*/
—	—	@@ -999,7 +1035,7 @@
1000	1036	/**
1001	1037	* Get the virtual URL for the file itself
1002	1038	*
1003		~~- * @param $suffix bool~~
	1039	+ * @param $suffix bool\|string if not false, the name of a thumbnail file
1004	1040	*
1005	1041	* @return string
1006	1042	*/

Sign-offs

User	Flag	Date
Reedy	inspected	11:43, 7 September 2011
Reedy	tested	11:43, 7 September 2011

Follow-up revisions

Revision	Commit summary	Author	Date
r96375	delete now-empty archived thumb directories	nelson	21:51, 6 September 2011
r96536	copyover: Don't ship with LocalSettings.php; Strip out secrets from proxy.co...	nelson	01:21, 8 September 2011
r96845	REL1_18: MFT r96373, r96399	reedy	14:47, 12 September 2011
r96846	Move RELEASE-NOTES from r96373 from RELEASE-NOTES-1.18 to RELEASE-NOTES-1.19 ...	reedy	14:49, 12 September 2011
r96856	1.17wmf1: MFT r94212, r96373, r96386, r96389, r96420, r96645...	reedy	15:35, 12 September 2011

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r94212	(bug 30192) Old thumbnails not properly purged. Unlike the bug suggests, we d...	demon	23:29, 10 August 2011

Status & tagging log

15:36, 12 September 2011 Reedy (talk | contribs) changed the tags for r96373 [removed: 1.17wmf1]
14:48, 12 September 2011 Reedy (talk | contribs) changed the tags for r96373 [removed: 1.18]
11:14, 9 September 2011 Reedy (talk | contribs) changed the status of r96373 [removed: new added: ok]
01:16, 7 September 2011 😂 (talk | contribs) changed the tags for r96373 [added: 1.17wmf1]
21:03, 6 September 2011 Reedy (talk | contribs) changed the tags for r96373 [added: 1.18]