1 files changed, 53 insertions, 35 deletions

diff --git a/maintenance/cleanupImages.php b/maintenance/cleanupImages.php
index 00903f22..db13f4c9 100644
--- a/maintenance/cleanupImages.php
+++ b/maintenance/cleanupImages.php
@@ -24,23 +24,26 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  * http://www.gnu.org/copyleft/gpl.html
  *
- * @file
  * @author Brion Vibber <brion at pobox.com>
  * @ingroup Maintenance
  */
 
-require_once( 'commandLine.inc' );
-require_once( 'cleanupTable.inc' );
+require_once( dirname(__FILE__) . '/cleanupTable.inc' );
 
-/**
- * @ingroup Maintenance
- */
 class ImageCleanup extends TableCleanup {
-	function __construct( $dryrun = false ) {
-		parent::__construct( 'image', $dryrun );
+	protected $defaultParams = array(
+		'table' => 'image',
+		'conds' => array(),
+		'index' => 'img_name',
+		'callback' => 'processRow',
+	);
+
+	public function __construct() {
+		parent::__construct();
+		$this->mDescription = "Script to clean up broken, unparseable upload filenames";
 	}
 
-	function processPage( $row ) {
+	protected function processRow( $row ) {
 		global $wgContLang;
 
 		$source = $row->img_name;
@@ -62,12 +65,12 @@ class ImageCleanup extends TableCleanup {
 		$cleaned = $wgContLang->checkTitleEncoding( $cleaned );
 		
 		// Many of remainder look like non-normalized unicode
-		$cleaned = UtfNormal::cleanUp( $cleaned );
+		$cleaned = $wgContLang->normalize( $cleaned );
 		
 		$title = Title::makeTitleSafe( NS_FILE, $cleaned );
 		
 		if( is_null( $title ) ) {
-			$this->log( "page $source ($cleaned) is illegal." );
+			$this->output( "page $source ($cleaned) is illegal.\n" );
 			$safe = $this->buildSafeTitle( $cleaned );
 			if( $safe === false )
 				return $this->progress( 0 );
@@ -77,19 +80,19 @@ class ImageCleanup extends TableCleanup {
 
 		if( $title->getDBkey() !== $source ) {
 			$munged = $title->getDBkey();
-			$this->log( "page $source ($munged) doesn't match self." );
+			$this->output( "page $source ($munged) doesn't match self.\n" );
 			$this->pokeFile( $source, $munged );
 			return $this->progress( 1 );
 		}
 
 		$this->progress( 0 );
 	}
-	
-	function killRow( $name ) {
+
+	private function killRow( $name ) {
 		if( $this->dryrun ) {
-			$this->log( "DRY RUN: would delete bogus row '$name'" );
+			$this->output( "DRY RUN: would delete bogus row '$name'\n" );
 		} else {
-			$this->log( "deleting bogus row '$name'" );
+			$this->output( "deleting bogus row '$name'\n" );
 			$db = wfGetDB( DB_MASTER );
 			$db->delete( 'image',
 				array( 'img_name' => $name ),
@@ -97,37 +100,55 @@ class ImageCleanup extends TableCleanup {
 		}
 	}
 	
-	function filePath( $name ) {
+	private function filePath( $name ) {
 		if ( !isset( $this->repo ) ) {
 			$this->repo = RepoGroup::singleton()->getLocalRepo();
 		}
 		return $this->repo->getRootDirectory() . '/' . $this->repo->getHashPath( $name ) . $name;
 	}
+
+	private function imageExists( $name, $db ) {
+		return $db->selectField( 'image', '1', array( 'img_name' => $name ), __METHOD__ );
+	}
+
+	private function pageExists( $name, $db ) {
+		return $db->selectField( 'page', '1', array( 'page_namespace' => NS_FILE, 'page_title' => $name ), __METHOD__ );
+	}
 	
-	function pokeFile( $orig, $new ) {
+	private function pokeFile( $orig, $new ) {
 		$path = $this->filePath( $orig );
 		if( !file_exists( $path ) ) {
-			$this->log( "missing file: $path" );
+			$this->output( "missing file: $path\n" );
 			return $this->killRow( $orig );
 		}
 		
 		$db = wfGetDB( DB_MASTER );
+
+		/*
+		 * To prevent key collisions in the update() statements below,
+		 * if the target title exists in the image table, or if both the
+		 * original and target titles exist in the page table, append
+		 * increasing version numbers until the target title exists in
+		 * neither.  (See also bug 16916.)
+		 */
 		$version = 0;
 		$final = $new;
+		$conflict = ( $this->imageExists( $final, $db ) ||
+			      ( $this->pageExists( $orig, $db ) && $this->pageExists( $final, $db ) ) );
 		
-		while( $db->selectField( 'image', 'img_name', array( 'img_name' => $final ), __METHOD__ ) ||
-			Title::makeTitle( NS_FILE, $final )->exists() ) {
-			$this->log( "Rename conflicts with '$final'..." );
+		while( $conflict ) {
+			$this->output( "Rename conflicts with '$final'...\n" );
 			$version++;
 			$final = $this->appendTitle( $new, "_$version" );
+			$conflict = ( $this->imageExists( $final, $db ) || $this->pageExists( $final, $db ) );
 		}
 		
 		$finalPath = $this->filePath( $final );
 		
 		if( $this->dryrun ) {
-			$this->log( "DRY RUN: would rename $path to $finalPath" );
+			$this->output( "DRY RUN: would rename $path to $finalPath\n" );
 		} else {
-			$this->log( "renaming $path to $finalPath" );
+			$this->output( "renaming $path to $finalPath\n" );
 			// XXX: should this use File::move()?  FIXME?
 			$db->begin();
 			$db->update( 'image',
@@ -153,18 +174,18 @@ class ImageCleanup extends TableCleanup {
 			if( rename( $path, $finalPath ) ) {
 				$db->commit();
 			} else {
-				$this->log( "RENAME FAILED" );
+				$this->error( "RENAME FAILED" );
 				$db->rollback();
 			}
 		}
 	}
-	
-	function appendTitle( $name, $suffix ) {
+
+	private function appendTitle( $name, $suffix ) {
 		return preg_replace( '/^(.*)(\..*?)$/',
 			"\\1$suffix\\2", $name );
 	}
-	
-	function buildSafeTitle( $name ) {
+
+	private function buildSafeTitle( $name ) {
 		global $wgLegalTitleChars;
 		$x = preg_replace_callback(
 			"/([^$wgLegalTitleChars]|~)/",
@@ -173,7 +194,7 @@ class ImageCleanup extends TableCleanup {
 		
 		$test = Title::makeTitleSafe( NS_FILE, $x );
 		if( is_null( $test ) || $test->getDBkey() !== $x ) {
-			$this->log( "Unable to generate safe title from '$name', got '$x'" );
+			$this->error( "Unable to generate safe title from '$name', got '$x'" );
 			return false;
 		}
 		
@@ -181,8 +202,5 @@ class ImageCleanup extends TableCleanup {
 	}
 }
 
-$wgUser->setName( 'Conversion script' );
-$caps = new ImageCleanup( !isset( $options['fix'] ) );
-$caps->cleanup();
-
-
+$maintClass = "ImageCleanup";
+require_once( DO_MAINTENANCE );