From 05cf8ae2164ef0ea7af891d344e093ae93f312c6 Mon Sep 17 00:00:00 2001 From: DannyS712 Date: Tue, 25 Feb 2020 22:33:18 +0000 Subject: [PATCH] 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 | 7 +++ includes/FileDeleteForm.php | 2 +- includes/filerepo/file/File.php | 23 ++++++++ includes/filerepo/file/ForeignDBFile.php | 11 ++++ includes/filerepo/file/LocalFile.php | 58 ++++++++++++++++++- .../filerepo/file/LocalFileDeleteBatch.php | 49 ++++++++++++++-- 6 files changed, 141 insertions(+), 9 deletions(-) diff --git a/RELEASE-NOTES-1.35 b/RELEASE-NOTES-1.35 index e8aaceedf75..210377301dc 100644 --- a/RELEASE-NOTES-1.35 +++ b/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. diff --git a/includes/FileDeleteForm.php b/includes/FileDeleteForm.php index 73cd93dcac2..1b2b1651517 100644 --- a/includes/FileDeleteForm.php +++ b/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(); diff --git a/includes/filerepo/file/File.php b/includes/filerepo/file/File.php index 6aecb58d0a6..fb935444e2e 100644 --- a/includes/filerepo/file/File.php +++ b/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. diff --git a/includes/filerepo/file/ForeignDBFile.php b/includes/filerepo/file/ForeignDBFile.php index 7edefd56ef8..b77eea5fc82 100644 --- a/includes/filerepo/file/ForeignDBFile.php +++ b/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 diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index 94d04f132b1..2bfdd78ad4e 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/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 ); diff --git a/includes/filerepo/file/LocalFileDeleteBatch.php b/includes/filerepo/file/LocalFileDeleteBatch.php index efd6776ac81..915a4ff0a39 100644 --- a/includes/filerepo/file/LocalFileDeleteBatch.php +++ b/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(); }