Do not pass $archiveName two times to OldLocalFile

Check the first dozen lines in ImportableUploadRevisionImporter::import().
In all cases the $archiveName is passed as a parameter to
OldLocalFile::newFromArchiveName(), and available via
OldLocalFile::getArchiveName().

I used https://codesearch.wmflabs.org/search/?q=uploadOld to make sure
this method is not called anywhere else. I consider it "package private"
and suggest to not apply the deprecation policy on this particular code.

Change-Id: Ibe9fb55d7e0d6e87698a56f7cfbf31d3d44ba70c

diff --git a/includes/filerepo/file/OldLocalFile.php b/includes/filerepo/file/OldLocalFile.php
index ad95bb4..23ff304 100644 (file)
--- a/includes/filerepo/file/OldLocalFile.php
+++ b/includes/filerepo/file/OldLocalFile.php
@@ -406,16 +406,15 @@ class OldLocalFile extends LocalFile {
         * Upload a file directly into archive. Generally for Special:Import.
         *
         * @param string $srcPath File system path of the source file
-        * @param string $archiveName Full archive name of the file, in the form
-        *   $timestamp!$filename, where $filename must match $this->getName()
         * @param string $timestamp
         * @param string $comment
         * @param User $user
         * @return Status
         */
-       function uploadOld( $srcPath, $archiveName, $timestamp, $comment, $user ) {
+       public function uploadOld( $srcPath, $timestamp, $comment, $user ) {
                $this->lock();
 
+               $archiveName = $this->getArchiveName();
                $dstRel = $this->getArchiveRel( $archiveName );
                $status = $this->publishTo( $srcPath, $dstRel );
 

diff --git a/includes/import/ImportableUploadRevisionImporter.php b/includes/import/ImportableUploadRevisionImporter.php
index 4fbddb5..4b378c1 100644 (file)
--- a/includes/import/ImportableUploadRevisionImporter.php
+++ b/includes/import/ImportableUploadRevisionImporter.php
@@ -104,9 +104,13 @@ class ImportableUploadRevisionImporter implements UploadRevisionImporter {
                        ?: User::newFromName( $importableRevision->getUser(), false );
 
                # Do the actual upload
-               if ( $archiveName ) {
-                       $status = $file->uploadOld( $source, $archiveName,
-                               $importableRevision->getTimestamp(), $importableRevision->getComment(), $user );
+               if ( $file instanceof OldLocalFile ) {
+                       $status = $file->uploadOld(
+                               $source,
+                               $importableRevision->getTimestamp(),
+                               $importableRevision->getComment(),
+                               $user
+                       );
                } else {
                        $flags = 0;
                        $status = $file->upload(