* Fixed bug 13532, incorrect use of wfFindFile() causing total breakage of file revert

* Use protected not private
* Remove aggressive history-oriented caching from OldLocalFile
* Keep a process cache of the OldLocalFile object to avoid duplicate queries

diff --git a/includes/FileRevertForm.php b/includes/FileRevertForm.php
index f335d02..b8455bb 100644 (file)
--- a/includes/FileRevertForm.php
+++ b/includes/FileRevertForm.php
@@ -8,10 +8,11 @@
  */
 class FileRevertForm {
 
-       private $title = null;
-       private $file = null;
-       private $oldimage = '';
-       private $timestamp = false;
+       protected $title = null;
+       protected $file = null;
+       protected $archiveName = '';
+       protected $timestamp = false;
+       protected $oldFile;
        
        /**
         * Constructor
@@ -48,10 +49,10 @@ class FileRevertForm {
                        return;
                }
                
-               $this->oldimage = $wgRequest->getText( 'oldimage' );
+               $this->archiveName = $wgRequest->getText( 'oldimage' );
                $token = $wgRequest->getText( 'wpEditToken' );
                if( !$this->isValidOldSpec() ) {
-                       $wgOut->showUnexpectedValueError( 'oldimage', htmlspecialchars( $this->oldimage ) );
+                       $wgOut->showUnexpectedValueError( 'oldimage', htmlspecialchars( $this->archiveName ) );
                        return;
                }
                
@@ -62,8 +63,8 @@ class FileRevertForm {
                }
                
                // Perform the reversion if appropriate
-               if( $wgRequest->wasPosted() && $wgUser->matchEditToken( $token, $this->oldimage ) ) {
-                       $source = $this->file->getArchiveVirtualUrl( $this->oldimage );
+               if( $wgRequest->wasPosted() && $wgUser->matchEditToken( $token, $this->archiveName ) ) {
+                       $source = $this->file->getArchiveVirtualUrl( $this->archiveName );
                        $comment = $wgRequest->getText( 'wpComment' );
                        // TODO: Preserve file properties from database instead of reloading from file
                        $status = $this->file->upload( $source, $comment, $comment );
@@ -71,7 +72,7 @@ class FileRevertForm {
                                $wgOut->addHtml( wfMsgExt( 'filerevert-success', 'parse', $this->title->getText(),
                                        $wgLang->date( $this->getTimestamp(), true ),
                                        $wgLang->time( $this->getTimestamp(), true ),
-                                       wfExpandUrl( $this->file->getArchiveUrl( $this->oldimage ) ) ) );
+                                       wfExpandUrl( $this->file->getArchiveUrl( $this->archiveName ) ) ) );
                                $wgOut->returnToMain( false, $this->title );
                        } else {
                                $wgOut->addWikiText( $status->getWikiText() );
@@ -86,16 +87,16 @@ class FileRevertForm {
        /**
         * Show the confirmation form
         */
-       private function showForm() {
+       protected function showForm() {
                global $wgOut, $wgUser, $wgRequest, $wgLang, $wgContLang;
                $timestamp = $this->getTimestamp();
 
                $form  = Xml::openElement( 'form', array( 'method' => 'post', 'action' => $this->getAction() ) );
-               $form .= Xml::hidden( 'wpEditToken', $wgUser->editToken( $this->oldimage ) );
+               $form .= Xml::hidden( 'wpEditToken', $wgUser->editToken( $this->archiveName ) );
                $form .= '<fieldset><legend>' . wfMsgHtml( 'filerevert-legend' ) . '</legend>';
                $form .= wfMsgExt( 'filerevert-intro', 'parse', $this->title->getText(),
                        $wgLang->date( $timestamp, true ), $wgLang->time( $timestamp, true ),
-                       wfExpandUrl( $this->file->getArchiveUrl( $this->oldimage ) ) );
+                       wfExpandUrl( $this->file->getArchiveUrl( $this->archiveName ) ) );
                $form .= '<p>' . Xml::inputLabel( wfMsg( 'filerevert-comment' ), 'wpComment', 'wpComment',
                        60, wfMsgForContent( 'filerevert-defaultcomment',
                        $wgContLang->date( $timestamp, false, false ), $wgContLang->time( $timestamp, false, false ) ) ) . '</p>';
@@ -109,7 +110,7 @@ class FileRevertForm {
        /**
         * Set headers, titles and other bits
         */
-       private function setHeaders() {
+       protected function setHeaders() {
                global $wgOut, $wgUser;
                $wgOut->setPageTitle( wfMsg( 'filerevert', $this->title->getText() ) );
                $wgOut->setRobotPolicy( 'noindex,nofollow' );
@@ -121,10 +122,10 @@ class FileRevertForm {
         *
         * @return bool
         */
-       private function isValidOldSpec() {
-               return strlen( $this->oldimage ) >= 16
-                       && strpos( $this->oldimage, '/' ) === false
-                       && strpos( $this->oldimage, '\\' ) === false;
+       protected function isValidOldSpec() {
+               return strlen( $this->archiveName ) >= 16
+                       && strpos( $this->archiveName, '/' ) === false
+                       && strpos( $this->archiveName, '\\' ) === false;
        }
        
        /**
@@ -133,9 +134,8 @@ class FileRevertForm {
         *
         * @return bool
         */
-       private function haveOldVersion() {
-               $file = wfFindFile( $this->title, $this->oldimage );
-               return $file && $file->exists() && $file->isLocal();
+       protected function haveOldVersion() {
+               return $this->getOldFile()->exists();
        }
        
        /**
@@ -143,10 +143,10 @@ class FileRevertForm {
         *
         * @return string
         */
-       private function getAction() {
+       protected function getAction() {
                $q = array();
                $q[] = 'action=revert';
-               $q[] = 'oldimage=' . urlencode( $this->oldimage );
+               $q[] = 'oldimage=' . urlencode( $this->archiveName );
                return $this->title->getLocalUrl( implode( '&', $q ) );
        }
        
@@ -155,12 +155,17 @@ class FileRevertForm {
         *
         * @return string
         */
-       private function getTimestamp() {
+       protected function getTimestamp() {
                if( $this->timestamp === false ) {
-                       $file = RepoGroup::singleton()->getLocalRepo()->newFromArchiveName( $this->title, $this->oldimage );
-                       $this->timestamp = $file->getTimestamp();
+                       $this->timestamp = $this->getOldFile()->getTimestamp();
                }
                return $this->timestamp;
        }
-       
-}
\ No newline at end of file
+
+       protected function getOldFile() {
+               if ( !isset( $this->oldFile ) ) {
+                       $this->oldFile = RepoGroup::singleton()->getLocalRepo()->newFromArchiveName( $this->title, $this->archiveName );
+               }
+               return $this->oldFile;
+       }
+}

diff --git a/includes/filerepo/OldLocalFile.php b/includes/filerepo/OldLocalFile.php
index 4906e79..5aebb6d 100644 (file)
--- a/includes/filerepo/OldLocalFile.php
+++ b/includes/filerepo/OldLocalFile.php
@@ -45,8 +45,7 @@ class OldLocalFile extends LocalFile {
        }
 
        function getCacheKey() {
-               $hashedName = md5($this->getName());
-               return wfMemcKey( 'oldfile', $hashedName );
+               return false;
        }
 
        function getArchiveName() {
@@ -64,105 +63,6 @@ class OldLocalFile extends LocalFile {
                return $this->exists() && !$this->isDeleted(File::DELETED_FILE);
        }
 
-       /**
-        * Try to load file metadata from memcached. Returns true on success.
-        */
-       function loadFromCache() {
-               global $wgMemc;
-               wfProfileIn( __METHOD__ );
-               $this->dataLoaded = false;
-               $key = $this->getCacheKey();
-               if ( !$key ) {
-                       return false;
-               }
-               $oldImages = $wgMemc->get( $key );
-
-               if ( isset( $oldImages['version'] ) && $oldImages['version'] == self::CACHE_VERSION ) {
-                       unset( $oldImages['version'] );
-                       $more = isset( $oldImages['more'] );
-                       unset( $oldImages['more'] );
-                       $found = false;
-                       if ( is_null( $this->requestedTime ) ) {
-                               foreach ( $oldImages as $timestamp => $info ) {
-                                       if ( $info['archive_name'] == $this->archive_name ) {
-                                               $found = true;
-                                               break;
-                                       }
-                               }
-                       } else {
-                               krsort( $oldImages );
-                               foreach ( $oldImages as $timestamp => $info ) {
-                                       if ( $timestamp == $this->requestedTime ) {
-                                               $found = true;
-                                               break;
-                                       }
-                               }
-                       }
-                       if ( $found ) {
-                               wfDebug( "Pulling file metadata from cache key {$key}[{$timestamp}]\n" );
-                               $this->dataLoaded = true;
-                               $this->fileExists = true;
-                               foreach ( $info as $name => $value ) {
-                                       $this->$name = $value;
-                               }
-                       } elseif ( $more ) {
-                               wfDebug( "Cache key was truncated, oldimage row might be found in the database\n" );
-                       } else {
-                               wfDebug( "Image did not exist at the specified time.\n" );
-                               $this->fileExists = false;
-                               $this->dataLoaded = true;
-                       }
-               }
-
-               if ( $this->dataLoaded ) {
-                       wfIncrStats( 'image_cache_hit' );
-               } else {
-                       wfIncrStats( 'image_cache_miss' );
-               }
-
-               wfProfileOut( __METHOD__ );
-               return $this->dataLoaded;
-       }
-
-       function saveToCache() {
-               // If a timestamp was specified, cache the entire history of the image (up to MAX_CACHE_ROWS).
-               if ( is_null( $this->requestedTime ) ) {
-                       return;
-               }
-               // This is expensive, so we only do it if $wgMemc is real
-               global $wgMemc;
-               if ( $wgMemc instanceof FakeMemcachedClient ) {
-                       return;
-               }
-               $key = $this->getCacheKey();
-               if ( !$key ) { 
-                       return;
-               }
-               wfProfileIn( __METHOD__ );
-
-               $dbr = $this->repo->getSlaveDB();
-               $res = $dbr->select( 'oldimage', $this->getCacheFields( 'oi_' ),
-                       array( 'oi_name' => $this->getName() ), __METHOD__, 
-                       array( 
-                               'LIMIT' => self::MAX_CACHE_ROWS + 1,
-                               'ORDER BY' => 'oi_timestamp DESC',
-                       ));
-               $cache = array( 'version' => self::CACHE_VERSION );
-               $numRows = $dbr->numRows( $res );
-               if ( $numRows > self::MAX_CACHE_ROWS ) {
-                       $cache['more'] = true;
-                       $numRows--;
-               }
-               for ( $i = 0; $i < $numRows; $i++ ) {
-                       $row = $dbr->fetchObject( $res );
-                       $decoded = $this->decodeRow( $row, 'oi_' );
-                       $cache[$row->oi_timestamp] = $decoded;
-               }
-               $dbr->freeResult( $res );
-               $wgMemc->set( $key, $cache, 7*86400 /* 1 week */ );
-               wfProfileOut( __METHOD__ );
-       }
-
        function loadFromDB() {
                wfProfileIn( __METHOD__ );
                $this->dataLoaded = true;