From cbf03f81d726d9cd9d44f0d3a5e7d71087652e12 Mon Sep 17 00:00:00 2001
From: addshore <addshorewiki@gmail.com>
Date: Mon, 8 May 2017 18:34:49 +0200
Subject: [PATCH] Refactor UploadBase::checkWarnings into smaller methods

These methods also don't access any of the class
properties and could one day be factored out into
some file checking service.

This also means that individual checks can be used for
the attached task if made protected.

Bug: T163500
Change-Id: I7cf912507ee02c35b6a666d7ed48fcab001316d3
---
 includes/upload/UploadBase.php | 154 ++++++++++++++++++++++++++-------
 1 file changed, 124 insertions(+), 30 deletions(-)

diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 57bb22ab96..631c4dc6e0 100644
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -641,48 +641,128 @@ abstract class UploadBase {
 	 *
 	 * This should not assume that mTempPath is set.
 	 *
-	 * @return array Array of warnings
+	 * @return mixed[] Array of warnings
 	 */
 	public function checkWarnings() {
-		global $wgLang;
-
 		$warnings = [];
 
 		$localFile = $this->getLocalFile();
 		$localFile->load( File::READ_LATEST );
 		$filename = $localFile->getName();
+		$hash = $this->getTempFileSha1Base36();
 
-		/**
-		 * Check whether the resulting filename is different from the desired one,
-		 * but ignore things like ucfirst() and spaces/underscore things
-		 */
-		$comparableName = str_replace( ' ', '_', $this->mDesiredDestName );
+		$badFileName = $this->checkBadFileName( $filename, $this->mDesiredDestName );
+		if ( $badFileName !== null ) {
+			$warnings['badfilename'] = $badFileName;
+		}
+
+		$unwantedFileExtensionDetails = $this->checkUnwantedFileExtensions( $this->mFinalExtension );
+		if ( $unwantedFileExtensionDetails !== null ) {
+			$warnings['filetype-unwanted-type'] = $unwantedFileExtensionDetails;
+		}
+
+		$fileSizeWarnings = $this->checkFileSize( $this->mFileSize );
+		if ( $fileSizeWarnings ) {
+			$warnings = array_merge( $warnings, $fileSizeWarnings );
+		}
+
+		$localFileExistsWarnings = $this->checkLocalFileExists( $localFile, $hash );
+		if ( $localFileExistsWarnings ) {
+			$warnings = array_merge( $warnings, $localFileExistsWarnings );
+		}
+
+		if ( $this->checkLocalFileWasDeleted( $localFile ) ) {
+			$warnings['was-deleted'] = $filename;
+		}
+
+		$dupes = $this->checkAgainstExistingDupes( $hash );
+		if ( $dupes ) {
+			$warnings['duplicate'] = $dupes;
+		}
+
+		$archivedDupes = $this->checkAgainstArchiveDupes( $hash );
+		if ( $archivedDupes !== null ) {
+			$warnings['duplicate-archive'] = $archivedDupes;
+		}
+
+		return $warnings;
+	}
+
+	/**
+	 * Check whether the resulting filename is different from the desired one,
+	 * but ignore things like ucfirst() and spaces/underscore things
+	 *
+	 * @param string $filename
+	 * @param string $desiredFileName
+	 *
+	 * @return string|null String that was determined to be bad or null if the filename is okay
+	 */
+	private function checkBadFileName( $filename, $desiredFileName ) {
+		$comparableName = str_replace( ' ', '_', $desiredFileName );
 		$comparableName = Title::capitalize( $comparableName, NS_FILE );
 
-		if ( $this->mDesiredDestName != $filename && $comparableName != $filename ) {
-			$warnings['badfilename'] = $filename;
+		if ( $desiredFileName != $filename && $comparableName != $filename ) {
+			return $filename;
 		}
 
-		// Check whether the file extension is on the unwanted list
-		global $wgCheckFileExtensions, $wgFileExtensions;
+		return null;
+	}
+
+	/**
+	 * @param string $fileExtension The file extension to check
+	 *
+	 * @return array|null array with the following keys:
+	 *                    0 => string The final extension being used
+	 *                    1 => string[] The extensions that are allowed
+	 *                    2 => int The number of extensions that are allowed.
+	 */
+	private function checkUnwantedFileExtensions( $fileExtension ) {
+		global $wgCheckFileExtensions, $wgFileExtensions, $wgLang;
+
 		if ( $wgCheckFileExtensions ) {
 			$extensions = array_unique( $wgFileExtensions );
-			if ( !$this->checkFileExtension( $this->mFinalExtension, $extensions ) ) {
-				$warnings['filetype-unwanted-type'] = [ $this->mFinalExtension,
-					$wgLang->commaList( $extensions ), count( $extensions ) ];
+			if ( !$this->checkFileExtension( $fileExtension, $extensions ) ) {
+				return [
+					$fileExtension,
+					$wgLang->commaList( $extensions ),
+					count( $extensions )
+				];
 			}
 		}
 
+		return null;
+	}
+
+	/**
+	 * @param int $fileSize
+	 *
+	 * @return array warnings
+	 */
+	private function checkFileSize( $fileSize ) {
 		global $wgUploadSizeWarning;
-		if ( $wgUploadSizeWarning && ( $this->mFileSize > $wgUploadSizeWarning ) ) {
-			$warnings['large-file'] = [ $wgUploadSizeWarning, $this->mFileSize ];
+
+		$warnings = [];
+
+		if ( $wgUploadSizeWarning && ( $fileSize > $wgUploadSizeWarning ) ) {
+			$warnings['large-file'] = [ $wgUploadSizeWarning, $fileSize ];
 		}
 
-		if ( $this->mFileSize == 0 ) {
+		if ( $fileSize == 0 ) {
 			$warnings['empty-file'] = true;
 		}
 
-		$hash = $this->getTempFileSha1Base36();
+		return $warnings;
+	}
+
+	/**
+	 * @param LocalFile $localFile
+	 * @param string $hash sha1 hash of the file to check
+	 *
+	 * @return array warnings
+	 */
+	private function checkLocalFileExists( LocalFile $localFile, $hash ) {
+		$warnings = [];
+
 		$exists = self::getExistsWarning( $localFile );
 		if ( $exists !== false ) {
 			$warnings['exists'] = $exists;
@@ -701,11 +781,19 @@ abstract class UploadBase {
 			}
 		}
 
-		if ( $localFile->wasDeleted() && !$localFile->exists() ) {
-			$warnings['was-deleted'] = $filename;
-		}
+		return $warnings;
+	}
+
+	private function checkLocalFileWasDeleted( LocalFile $localFile ) {
+		return $localFile->wasDeleted() && !$localFile->exists();
+	}
 
-		// Check dupes against existing files
+	/**
+	 * @param string $hash sha1 hash of the file to check
+	 *
+	 * @return File[] Duplicate files, if found.
+	 */
+	private function checkAgainstExistingDupes( $hash ) {
 		$dupes = RepoGroup::singleton()->findBySha1( $hash );
 		$title = $this->getTitle();
 		// Remove all matches against self
@@ -714,21 +802,27 @@ abstract class UploadBase {
 				unset( $dupes[$key] );
 			}
 		}
-		if ( $dupes ) {
-			$warnings['duplicate'] = $dupes;
-		}
 
-		// Check dupes against archives
+		return $dupes;
+	}
+
+	/**
+	 * @param string $hash sha1 hash of the file to check
+	 *
+	 * @return string|null Name of the dupe or empty string if discovered (depending on visibility)
+	 *                     null if the check discovered no dupes.
+	 */
+	private function checkAgainstArchiveDupes( $hash ) {
 		$archivedFile = new ArchivedFile( null, 0, '', $hash );
 		if ( $archivedFile->getID() > 0 ) {
 			if ( $archivedFile->userCan( File::DELETED_FILE ) ) {
-				$warnings['duplicate-archive'] = $archivedFile->getName();
+				return $archivedFile->getName();
 			} else {
-				$warnings['duplicate-archive'] = '';
+				return '';
 			}
 		}
 
-		return $warnings;
+		return null;
 	}
 
 	/**
-- 
2.20.1