Do not create new archive file names for old files
authorThiemo Kreuz <thiemo.kreuz@wikimedia.de>
Wed, 2 Jan 2019 14:34:59 +0000 (15:34 +0100)
committerMarostegui <marostegui@wikimedia.org>
Thu, 21 Mar 2019 10:25:21 +0000 (10:25 +0000)
When importing a previously archived file revision, \OldLocalFile::uploadOld()
calls \LocalFile::publishTo() with $dstRel pointing to a location in the
archive. It does not make sense to create a new archive file name for a file
that is already in the archive. Instead, use the existing archive file name.

* Note how $archiveName is not used in the code below, except as part of the
  returned status.
* $archiveRel ends in \FileRepo::publishBatch(), but is barely used there
  except for (again) the status.
* \FileRepo::publishBatch() makes use of the dirname() extracted from
  $archiveRel. This patch does not make changes to this path, only to the
  file name.

This is the most trivial patch we could think of to fix the bug that the
return value is not the documented one ("On success, the value member
contains the archive name").

This will be covered by the test introduced in I15fad26.

Bug: T200001
Bug: T210755
Change-Id: I28b782e9b41ed78ac1674111094335849e15ba49

includes/filerepo/file/LocalFile.php

index a3d1624..9b9e748 100644 (file)
@@ -1838,8 +1838,13 @@ class LocalFile extends File {
 
                $this->lock();
 
-               $archiveName = wfTimestamp( TS_MW ) . '!' . $this->getName();
-               $archiveRel = $this->getArchiveRel( $archiveName );
+               if ( $this->isOld() ) {
+                       $archiveRel = $dstRel;
+                       $archiveName = basename( $archiveRel );
+               } else {
+                       $archiveName = wfTimestamp( TS_MW ) . '!' . $this->getName();
+                       $archiveRel = $this->getArchiveRel( $archiveName );
+               }
 
                if ( $repo->hasSha1Storage() ) {
                        $sha1 = FileRepo::isVirtualUrl( $srcPath )