LocalFileDeleteBatch: Migrate to new signature requiring a user

Move the user parameter to be before the other optional parameters,
and hard deprecate not passing a user. Interim signature is fully
backwards compatible, emitting deprecation noticed when a user is
not passed but otherwise accepting parameters in the old order as well
as the old order, including handling of defaults.

File::delete is a soft deprecated wrapper for the new File::deleteFile,
and File::deleteOld is a hard deprecated wrapper for the new
File::deleteOldFile. Both of the former methods accepted optional user
parameters; both of the new methods require a user, so that the call to
construct a LocalFileDeleteBatch can pass a user.

Bug: T245710
Change-Id: I9cde7831e16a719c22f093f95248b8055d9ac6fd

RELEASE-NOTES-1.35

@@ -579,6 +579,13 @@ because of Phabricator reports.
   - ArchivedFile::userCan
 * Article::insertProtectNullRevision is deprecated. Instead, use
   WikiPage::insertProtectNullRevision.
+* LocalFileDeleteBatch was migrated to a new constructor signature with the
+  user as the second parameter. Support for the old signature is hard
+  deprecated, and once removed the user parameter will be required. At the
+  same time, a number of file-deletion related methods were updated
+  - File::delete is soft deprecated in favor of the new ::deleteFile
+  - LocalFile::delete is soft deprecated in favor of the new ::deleteFile
+  - LocalFile::deleteOld is hard deprecated in favor of the new ::deleteOldFile
 * The SpecialPageFactory class was moved from the MediaWiki\Special namespace
   to the MediaWiki\SpecialPage namespace. The old location remains as a
   deprecated alias.

includes/FileDeleteForm.php

@@ -196,7 +196,7 @@ class FileDeleteForm {
 
 		if ( $oldimage ) {
 			$page = null;
-			$status = $file->deleteOld( $oldimage, $reason, $suppress, $user );
+			$status = $file->deleteOldFile( $oldimage, $reason, $user, $suppress );
 			if ( $status->isOK() ) {
 				// Need to do a log item
 				$logComment = wfMessage( 'deletedrevision', $oldimage )->inContentLanguage()->text();

includes/filerepo/file/File.php

@@ -1980,6 +1980,8 @@ abstract class File implements IDBAccessObject {
 	/**
 	 * Delete all versions of the file.
 	 *
+	 * @deprecated since 1.35, use deleteFile instead
+	 *
 	 * Moves the files into an archive directory (or deletes them)
 	 * and removes the database rows.
 	 *
@@ -1996,6 +1998,27 @@ abstract class File implements IDBAccessObject {
 		$this->readOnlyError();
 	}
 
+	/**
+	 * Delete all versions of the file.
+	 *
+	 * @since 1.35
+	 *
+	 * Moves the files into an archive directory (or deletes them)
+	 * and removes the database rows.
+	 *
+	 * Cache purging is done; logging is caller's responsibility.
+	 *
+	 * @param string $reason
+	 * @param User $user
+	 * @param bool $suppress Hide content from sysops?
+	 * @return Status
+	 * STUB
+	 * Overridden by LocalFile
+	 */
+	function deleteFile( $reason, User $user, $suppress = false ) {
+		$this->readOnlyError();
+	}
+
 	/**
 	 * Restore all or specified deleted revisions to the given file.
 	 * Permissions and logging are left to the caller.

includes/filerepo/file/ForeignDBFile.php

@@ -81,6 +81,17 @@ class ForeignDBFile extends LocalFile {
 		$this->readOnlyError();
 	}
 
+	/**
+	 * @param string $reason
+	 * @param User $user
+	 * @param bool $suppress
+	 * @return Status
+	 * @throws MWException
+	 */
+	function deleteFile( $reason, User $user, $suppress = false ) {
+		$this->readOnlyError();
+	}
+
 	/**
 	 * @param Title $target
 	 * @return Status

includes/filerepo/file/LocalFile.php

@@ -1921,6 +1921,8 @@ class LocalFile extends File {
 	/**
 	 * Delete all versions of the file.
 	 *
+	 * @deprecated since 1.35, use deleteFile
+	 *
 	 * Moves the files into an archive directory (or deletes them)
 	 * and removes the database rows.
 	 *
@@ -1932,11 +1934,35 @@ class LocalFile extends File {
 	 * @return Status
 	 */
 	function delete( $reason, $suppress = false, $user = null ) {
+		// TODO check callers and hard deprecate
+		if ( $user === null ) {
+			global $wgUser;
+			$user = $wgUser;
+		}
+		return $this->deleteFile( $reason, $user, $suppress );
+	}
+
+	/**
+	 * Delete all versions of the file.
+	 *
+	 * @since 1.35
+	 *
+	 * Moves the files into an archive directory (or deletes them)
+	 * and removes the database rows.
+	 *
+	 * Cache purging is done; logging is caller's responsibility.
+	 *
+	 * @param string $reason
+	 * @param User $user
+	 * @param bool $suppress
+	 * @return Status
+	 */
+	function deleteFile( $reason, User $user, $suppress = false ) {
 		if ( $this->getRepo()->getReadOnlyReason() !== false ) {
 			return $this->readOnlyFatalStatus();
 		}
 
-		$batch = new LocalFileDeleteBatch( $this, $reason, $suppress, $user );
+		$batch = new LocalFileDeleteBatch( $this, $user, $reason, $suppress );
 
 		$this->lock();
 		$batch->addCurrent();
@@ -1977,6 +2003,8 @@ class LocalFile extends File {
 	/**
 	 * Delete an old version of the file.
 	 *
+	 * @deprecated since 1.35, use deleteOldFile
+	 *
 	 * Moves the file into an archive directory (or deletes it)
 	 * and removes the database row.
 	 *
@@ -1990,11 +2018,37 @@ class LocalFile extends File {
 	 * @return Status
 	 */
 	function deleteOld( $archiveName, $reason, $suppress = false, $user = null ) {
+		wfDeprecated( __METHOD__, '1.35' );
+		if ( $user === null ) {
+			global $wgUser;
+			$user = $wgUser;
+		}
+		return $this->deleteOldFile( $archiveName, $reason, $user, $suppress );
+	}
+
+	/**
+	 * Delete an old version of the file.
+	 *
+	 * @since 1.35
+	 *
+	 * Moves the file into an archive directory (or deletes it)
+	 * and removes the database row.
+	 *
+	 * Cache purging is done; logging is caller's responsibility.
+	 *
+	 * @param string $archiveName
+	 * @param string $reason
+	 * @param User $user
+	 * @param bool $suppress
+	 * @throws MWException Exception on database or file store failure
+	 * @return Status
+	 */
+	function deleteOldFile( $archiveName, $reason, User $user, $suppress = false ) {
 		if ( $this->getRepo()->getReadOnlyReason() !== false ) {
 			return $this->readOnlyFatalStatus();
 		}
 
-		$batch = new LocalFileDeleteBatch( $this, $reason, $suppress, $user );
+		$batch = new LocalFileDeleteBatch( $this, $user, $reason, $suppress );
 
 		$this->lock();
 		$batch->addOld( $archiveName );

includes/filerepo/file/LocalFileDeleteBatch.php

@@ -55,16 +55,53 @@ class LocalFileDeleteBatch {
 
 	/**
 	 * @param File $file
-	 * @param string $reason
-	 * @param bool $suppress
-	 * @param User|null $user
+	 * @param string|User $param2
+	 * @param bool|string $param3
+	 * @param User|null|bool $param4
+	 *
+	 * Old signature: $file, $reason = '', $suppress = false, $user = null
+	 * New signature: $file, $user, $reason = '', $suppress = false
+	 *
+	 * See T245710 for more
 	 */
-	public function __construct( File $file, $reason = '', $suppress = false, $user = null ) {
+	public function __construct(
+		File $file,
+		$param2 = '',
+		$param3 = '',
+		$param4 = false
+	) {
 		$this->file = $file;
+
+		if ( $param2 instanceof User ) {
+			// New signature
+			$user = $param2;
+			$reason = $param3;
+			$suppress = $param4;
+		} else {
+			// Old signature
+			wfDeprecated(
+				__CLASS__ .
+				' being constructed without passing a user as the second parameter.' .
+				' See T245710 for more',
+				'1.35'
+			);
+
+			$reason = $param2;
+
+			// Suppress defaults to false if not provided
+			$suppress = ( $param3 === '' ? false : $param3 );
+
+			if ( $param4 === false ) {
+				global $wgUser;
+				$user = $wgUser;
+			} else {
+				$user = $param4;
+			}
+		}
+
 		$this->reason = $reason;
 		$this->suppress = $suppress;
-		global $wgUser;
-		$this->user = $user ?: $wgUser;
+		$this->user = $user;
 		$this->status = $file->repo->newGood();
 	}