r112151 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r112150‎ | r112151 | r112152 >
Date:21:24, 22 February 2012
Author:catrope
Status:ok (Comments)
Tags:
Comment:
1.19wmf1: Fix the UploadStash hack in thumb.php so it works with the FileRepo refactoring. This is already live.
* FSRepo constructor needs real file paths, but getZonePath() returns mwstore paths. Pass in data from $wgLocalFileRepo instead. This is a hack but whatever
* FSRepo constructor requires a 'name' key, pass one. This caused a notice in 1.18wmf1 but causes an exception in 1.19wmf1
* For building $path, use the 'public' zone of the fake repo rather than the 'temp' zone of the real repo. They have the same FS paths, so they were identical in 1.18wmf1 but the mwstore URLs returned in 1.19wmf1 are different
* Pass a Title object with $strippedName into the UnregisteredLocalFile constructor so $this->name is set to $strippedName rather than $fileName, which is needed for hash paths to be computed correctly (the hash path for 20120222132345!foo.png should be based on md5('foo.png'), not on md5('20120222132345!foo.png') )
Modified paths:
  • /branches/wmf/1.19wmf1/thumb.php (modified) (history)

Diff [purge]

Index: branches/wmf/1.19wmf1/thumb.php
——@@ -125,19 +125,22 @@
126126 // Dirty, horrible, evil hack. We need to create a repo with the right zone paths for this to work
127127 $localRepo = RepoGroup::singleton()->getLocalRepo();
128128
 129+ global $wgLocalFileRepo;
129130 $repo = new FSRepo(
130 - array( 'directory' => $localRepo->getZonePath( 'temp' ),
131 - 'url' => $localRepo->getZoneUrl( 'temp' ),
132 - 'thumbDir'=> $localRepo->getZonePath( 'thumb' ) . '/temp',
133 - 'thumbUrl' => $localRepo->getZoneUrl( 'thumb' ) . '/temp'
 131+ array( 'directory' => $wgLocalFileRepo['directory'] . '/temp',
 132+ 'url' => $wgLocalFileRepo['url'] . '/temp',
 133+ 'thumbDir'=> $wgLocalFileRepo['thumbDir'] . '/temp',
 134+ 'thumbUrl' => $wgLocalFileRepo['thumbUrl'] . '/temp',
 135+ 'name' => 'UploadStashHackery',
134136 )
135137 );
136138
137139 // $fileName can be like timestamp!name , strip the timestamp! part
138140 $parts = explode( '!', $fileName, 2 );
139141 $strippedName = isset( $parts[1] ) ? $parts[1] : $fileName;
140 - $path = $localRepo->getZonePath( 'temp' ) . '/' . RepoGroup::singleton()->getLocalRepo()->getHashPath( $strippedName ) . $fileName;
141 - $img = new UnregisteredLocalFile( false, $repo, $path, false );
 142+ $path = $repo->getZonePath( 'public' ) . '/' . RepoGroup::singleton()->getLocalRepo()->getHashPath( $strippedName ) . $fileName;
 143+ // Need to pass in $strippedName as a Title object so hash paths are computed correctly
 144+ $img = new UnregisteredLocalFile( Title::makeTitle( NS_FILE, $strippedName ), $repo, $path, false );
142145 } else {
143146 $img = wfLocalFile( $fileName );
144147 }

Comments

#Comment by Aaron Schulz (talk | contribs)   21:33, 22 February 2012

The change to $repo->getZonePath( 'public' ) makes sense since the backend name portion of the path needs to match up with the repo's backend (e.g. $repo->getBackend()). If you give a path like mwstore://Y/cont/stuff.jpg to a backend named Y it will work, but it will be seen as invalid for a backend named X.

#Comment by Catrope (talk | contribs)   21:37, 22 February 2012

Yeah. It didn't matter before because it was all just FS paths anyway, so $repo->getZonePath( 'public' ) and $localRepo->getZonePath( 'temp' ) were the same string.

Status & tagging log