Aaron Schulz has uploaded a new change for review.
https://gerrit.wikimedia.org/r/314457
Change subject: Fix numerous FileRepo IDEA warnings
......................................................................
Fix numerous FileRepo IDEA warnings
Change-Id: I3522f37b675efffb68dec9d125faacd8c8776e64
---
M includes/filerepo/FileRepo.php
M includes/filerepo/ForeignDBRepo.php
M includes/filerepo/LocalRepo.php
M includes/filerepo/file/File.php
M includes/filerepo/file/ForeignDBFile.php
M includes/filerepo/file/LocalFile.php
M includes/filerepo/file/OldLocalFile.php
7 files changed, 58 insertions(+), 53 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/57/314457/1
diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php
index 1a6c818..66dab99 100644
--- a/includes/filerepo/FileRepo.php
+++ b/includes/filerepo/FileRepo.php
@@ -393,7 +393,7 @@
if ( $this->oldFileFactory ) {
return call_user_func( $this->oldFileFactory,
$title, $this, $time );
} else {
- return false;
+ return null;
}
} else {
return call_user_func( $this->fileFactory, $title,
$this );
@@ -818,7 +818,7 @@
* self::OVERWRITE_SAME Overwrite the file if the destination
exists and has the
* same contents as the source
* self::SKIP_LOCKING Skip any file locking when doing the store
- * @return FileRepoStatus
+ * @return Status
*/
public function store( $srcPath, $dstZone, $dstRel, $flags = 0 ) {
$this->assertWritableRepo(); // fail out if read-only
@@ -841,7 +841,7 @@
* same contents as the source
* self::SKIP_LOCKING Skip any file locking when doing the store
* @throws MWException
- * @return FileRepoStatus
+ * @return Status
*/
public function storeBatch( array $triplets, $flags = 0 ) {
$this->assertWritableRepo(); // fail out if read-only
@@ -912,7 +912,7 @@
* @param array $files List of files to delete
* @param int $flags Bitwise combination of the following flags:
* self::SKIP_LOCKING Skip any file locking when doing the
deletions
- * @return FileRepoStatus
+ * @return Status
*/
public function cleanupBatch( array $files, $flags = 0 ) {
$this->assertWritableRepo(); // fail out if read-only
@@ -952,7 +952,7 @@
* @param array|string|null $options An array consisting of a key named
headers
* listing extra headers. If a string, taken as content-disposition
header.
* (Support for array of options new in 1.23)
- * @return FileRepoStatus
+ * @return Status
*/
final public function quickImport( $src, $dst, $options = null ) {
return $this->quickImportBatch( [ [ $src, $dst, $options ] ] );
@@ -964,7 +964,7 @@
* This is intended for purging thumbnails.
*
* @param string $path Virtual URL or storage path
- * @return FileRepoStatus
+ * @return Status
*/
final public function quickPurge( $path ) {
return $this->quickPurgeBatch( [ $path ] );
@@ -995,7 +995,7 @@
* When "headers" are given they are used as HTTP headers if supported.
*
* @param array $triples List of (source path or FSFile, destination
path, disposition)
- * @return FileRepoStatus
+ * @return Status
*/
public function quickImportBatch( array $triples ) {
$status = $this->newGood();
@@ -1040,7 +1040,7 @@
* This does no locking nor journaling and is intended for purging
thumbnails.
*
* @param array $paths List of virtual URLs or storage paths
- * @return FileRepoStatus
+ * @return Status
*/
public function quickPurgeBatch( array $paths ) {
$status = $this->newGood();
@@ -1065,7 +1065,7 @@
* @param string $originalName The base name of the file as specified
* by the user. The file extension will be maintained.
* @param string $srcPath The current location of the file.
- * @return FileRepoStatus Object with the URL in the value.
+ * @return Status Object with the URL in the value.
*/
public function storeTemp( $originalName, $srcPath ) {
$this->assertWritableRepo(); // fail out if read-only
@@ -1107,7 +1107,7 @@
* @param string $dstPath Target file system path
* @param int $flags Bitwise combination of the following flags:
* self::DELETE_SOURCE Delete the source files on success
- * @return FileRepoStatus
+ * @return Status
*/
public function concatenate( array $srcPaths, $dstPath, $flags = 0 ) {
$this->assertWritableRepo(); // fail out if read-only
@@ -1156,7 +1156,7 @@
* @param int $flags Bitfield, may be FileRepo::DELETE_SOURCE to
indicate
* that the source file should be deleted if possible
* @param array $options Optional additional parameters
- * @return FileRepoStatus
+ * @return Status
*/
public function publish(
$src, $dstRel, $archiveRel, $flags = 0, array $options = []
@@ -1185,7 +1185,7 @@
* @param int $flags Bitfield, may be FileRepo::DELETE_SOURCE to
indicate
* that the source files should be deleted if possible
* @throws MWException
- * @return FileRepoStatus
+ * @return Status
*/
public function publishBatch( array $ntuples, $flags = 0 ) {
$this->assertWritableRepo(); // fail out if read-only
@@ -1322,7 +1322,10 @@
$params = [ 'noAccess' => true, 'noListing' => true ] +
$params;
}
- return $this->backend->prepare( $params );
+ $status = $this->newGood();
+ $status->merge( $this->backend->prepare( $params ) );
+
+ return $status;
}
/**
@@ -1380,7 +1383,7 @@
* @param mixed $srcRel Relative path for the file to be deleted
* @param mixed $archiveRel Relative path for the archive location.
* Relative to a private archive directory.
- * @return FileRepoStatus
+ * @return Status
*/
public function delete( $srcRel, $archiveRel ) {
$this->assertWritableRepo(); // fail out if read-only
@@ -1403,7 +1406,7 @@
* public root in the first element, and the archive file path
relative
* to the deleted zone root in the second element.
* @throws MWException
- * @return FileRepoStatus
+ * @return Status
*/
public function deleteBatch( array $sourceDestPairs ) {
$this->assertWritableRepo(); // fail out if read-only
@@ -1599,7 +1602,10 @@
$path = $this->resolveToStoragePath( $virtualUrl );
$params = [ 'src' => $path, 'headers' => $headers, 'options' =>
$optHeaders ];
- return $this->backend->streamFile( $params );
+ $status = $this->newGood();
+ $status->merge( $this->backend->streamFile( $params ) );
+
+ return $status;
}
/**
diff --git a/includes/filerepo/ForeignDBRepo.php
b/includes/filerepo/ForeignDBRepo.php
index be046bd..7fb7a0e 100644
--- a/includes/filerepo/ForeignDBRepo.php
+++ b/includes/filerepo/ForeignDBRepo.php
@@ -51,9 +51,12 @@
/** @var bool */
protected $hasSharedCache;
- # Other stuff
+ /** @var IDatabase */
protected $dbConn;
+
+ /** @var callable */
protected $fileFactory = [ 'ForeignDBFile', 'newFromTitle' ];
+ /** @var callable */
protected $fileFromRowFactory = [ 'ForeignDBFile', 'newFromRow' ];
/**
diff --git a/includes/filerepo/LocalRepo.php b/includes/filerepo/LocalRepo.php
index 21492a5..c195241 100644
--- a/includes/filerepo/LocalRepo.php
+++ b/includes/filerepo/LocalRepo.php
@@ -29,28 +29,24 @@
* @ingroup FileRepo
*/
class LocalRepo extends FileRepo {
- /** @var array */
+ /** @var callable */
protected $fileFactory = [ 'LocalFile', 'newFromTitle' ];
-
- /** @var array */
+ /** @var callable */
protected $fileFactoryKey = [ 'LocalFile', 'newFromKey' ];
-
- /** @var array */
+ /** @var callable */
protected $fileFromRowFactory = [ 'LocalFile', 'newFromRow' ];
-
- /** @var array */
+ /** @var callable */
protected $oldFileFromRowFactory = [ 'OldLocalFile', 'newFromRow' ];
-
- /** @var array */
+ /** @var callable */
protected $oldFileFactory = [ 'OldLocalFile', 'newFromTitle' ];
-
- /** @var array */
+ /** @var callable */
protected $oldFileFactoryKey = [ 'OldLocalFile', 'newFromKey' ];
function __construct( array $info = null ) {
parent::__construct( $info );
- $this->hasSha1Storage = isset( $info['storageLayout'] ) &&
$info['storageLayout'] === 'sha1';
+ $this->hasSha1Storage = isset( $info['storageLayout'] )
+ && $info['storageLayout'] === 'sha1';
if ( $this->hasSha1Storage() ) {
$this->backend = new FileBackendDBRepoWrapper( [
@@ -93,7 +89,7 @@
*
* @param array $storageKeys
*
- * @return FileRepoStatus
+ * @return Status
*/
function cleanupDeletedBatch( array $storageKeys ) {
if ( $this->hasSha1Storage() ) {
@@ -454,7 +450,7 @@
/**
* Get a connection to the replica DB
- * @return Database
+ * @return IDatabase
*/
function getSlaveDB() {
return wfGetDB( DB_REPLICA );
@@ -462,7 +458,7 @@
/**
* Get a connection to the master DB
- * @return Database
+ * @return IDatabase
*/
function getMasterDB() {
return wfGetDB( DB_MASTER );
@@ -562,7 +558,7 @@
*
* @param string $function
* @param array $args
- * @return FileRepoStatus
+ * @return Status
*/
protected function skipWriteOperationIfSha1( $function, array $args ) {
$this->assertWritableRepo(); // fail out if read-only
diff --git a/includes/filerepo/file/File.php b/includes/filerepo/file/File.php
index c48866b..c1d5573 100644
--- a/includes/filerepo/file/File.php
+++ b/includes/filerepo/file/File.php
@@ -1805,7 +1805,7 @@
* @param int $flags A bitwise combination of:
* File::DELETE_SOURCE Delete the source file, i.e. move rather
than copy
* @param array $options Optional additional parameters
- * @return FileRepoStatus On success, the value member contains the
+ * @return Status On success, the value member contains the
* archive name, or an empty string if it was a new file.
*
* STUB
@@ -1905,7 +1905,7 @@
* and logging are caller's responsibility
*
* @param Title $target New file name
- * @return FileRepoStatus
+ * @return Status
*/
function move( $target ) {
$this->readOnlyError();
@@ -1922,7 +1922,7 @@
* @param string $reason
* @param bool $suppress Hide content from sysops?
* @param User|null $user
- * @return FileRepoStatus
+ * @return Status
* STUB
* Overridden by LocalFile
*/
diff --git a/includes/filerepo/file/ForeignDBFile.php
b/includes/filerepo/file/ForeignDBFile.php
index cf0045e4..c041dea 100644
--- a/includes/filerepo/file/ForeignDBFile.php
+++ b/includes/filerepo/file/ForeignDBFile.php
@@ -57,7 +57,7 @@
* @param string $srcPath
* @param int $flags
* @param array $options
- * @return FileRepoStatus
+ * @return Status
* @throws MWException
*/
function publish( $srcPath, $flags = 0, array $options = [] ) {
@@ -84,7 +84,7 @@
/**
* @param array $versions
* @param bool $unsuppress
- * @return FileRepoStatus
+ * @return Status
* @throws MWException
*/
function restore( $versions = [], $unsuppress = false ) {
@@ -95,7 +95,7 @@
* @param string $reason
* @param bool $suppress
* @param User|null $user
- * @return FileRepoStatus
+ * @return Status
* @throws MWException
*/
function delete( $reason, $suppress = false, $user = null ) {
@@ -104,7 +104,7 @@
/**
* @param Title $target
- * @return FileRepoStatus
+ * @return Status
* @throws MWException
*/
function move( $target ) {
diff --git a/includes/filerepo/file/LocalFile.php
b/includes/filerepo/file/LocalFile.php
index 7ffb147..9df9360 100644
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -1160,7 +1160,7 @@
* @param User|null $user User object or null to use $wgUser
* @param string[] $tags Change tags to add to the log entry and page
revision.
* (This doesn't check $user's permissions.)
- * @return FileRepoStatus On success, the value member contains the
+ * @return Status On success, the value member contains the
* archive name, or an empty string if it was a new file.
*/
function upload( $src, $comment, $pageText, $flags = 0, $props = false,
@@ -1582,7 +1582,7 @@
* @param int $flags A bitwise combination of:
* File::DELETE_SOURCE Delete the source file, i.e. move rather
than copy
* @param array $options Optional additional parameters
- * @return FileRepoStatus On success, the value member contains the
+ * @return Status On success, the value member contains the
* archive name, or an empty string if it was a new file.
*/
function publish( $src, $flags = 0, array $options = [] ) {
@@ -1601,7 +1601,7 @@
* @param int $flags A bitwise combination of:
* File::DELETE_SOURCE Delete the source file, i.e. move rather
than copy
* @param array $options Optional additional parameters
- * @return FileRepoStatus On success, the value member contains the
+ * @return Status On success, the value member contains the
* archive name, or an empty string if it was a new file.
*/
function publishTo( $src, $dstRel, $flags = 0, array $options = [] ) {
@@ -1663,7 +1663,7 @@
* and logging are caller's responsibility
*
* @param Title $target New file name
- * @return FileRepoStatus
+ * @return Status
*/
function move( $target ) {
if ( $this->getRepo()->getReadOnlyReason() !== false ) {
@@ -1722,7 +1722,7 @@
* @param string $reason
* @param bool $suppress
* @param User|null $user
- * @return FileRepoStatus
+ * @return Status
*/
function delete( $reason, $suppress = false, $user = null ) {
if ( $this->getRepo()->getReadOnlyReason() !== false ) {
@@ -1780,7 +1780,7 @@
* @param bool $suppress
* @param User|null $user
* @throws MWException Exception on database or file store failure
- * @return FileRepoStatus
+ * @return Status
*/
function deleteOld( $archiveName, $reason, $suppress = false, $user =
null ) {
if ( $this->getRepo()->getReadOnlyReason() !== false ) {
@@ -1816,7 +1816,7 @@
* @param array $versions Set of record ids of deleted items to restore,
* or empty to restore all revisions.
* @param bool $unsuppress
- * @return FileRepoStatus
+ * @return Status
*/
function restore( $versions = [], $unsuppress = false ) {
if ( $this->getRepo()->getReadOnlyReason() !== false ) {
@@ -2346,7 +2346,7 @@
/**
* Run the transaction
- * @return FileRepoStatus
+ * @return Status
*/
public function execute() {
$repo = $this->file->getRepo();
@@ -2494,7 +2494,7 @@
* rows and there's no need to keep the image row locked while it's
acquiring those locks
* The caller may have its own transaction open.
* So we save the batch and let the caller call cleanup()
- * @return FileRepoStatus
+ * @return Status
*/
public function execute() {
/** @var Language */
@@ -2795,7 +2795,7 @@
/**
* Delete unused files in the deleted zone.
* This should be called from outside the transaction in which
execute() was called.
- * @return FileRepoStatus
+ * @return Status
*/
public function cleanup() {
if ( !$this->cleanupBatch ) {
@@ -2930,7 +2930,7 @@
/**
* Perform the move.
- * @return FileRepoStatus
+ * @return Status
*/
public function execute() {
$repo = $this->file->repo;
@@ -3002,7 +3002,7 @@
* Verify the database updates and return a new FileRepoStatus
indicating how
* many rows would be updated.
*
- * @return FileRepoStatus
+ * @return Status
*/
protected function verifyDBUpdates() {
$repo = $this->file->repo;
diff --git a/includes/filerepo/file/OldLocalFile.php
b/includes/filerepo/file/OldLocalFile.php
index 31e62ec..a17ca6e 100644
--- a/includes/filerepo/file/OldLocalFile.php
+++ b/includes/filerepo/file/OldLocalFile.php
@@ -332,7 +332,7 @@
* @param string $timestamp
* @param string $comment
* @param User $user
- * @return FileRepoStatus
+ * @return Status
*/
function uploadOld( $srcPath, $archiveName, $timestamp, $comment, $user
) {
$this->lock();
--
To view, visit https://gerrit.wikimedia.org/r/314457
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3522f37b675efffb68dec9d125faacd8c8776e64
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
