From: Tim Starling Date: Sat, 5 Apr 2008 16:46:22 +0000 (+0000) Subject: * Fixed bug 13532, incorrect use of wfFindFile() causing total breakage of file revert X-Git-Tag: 1.31.0-rc.0~48605 X-Git-Url: https://git.cyclocoop.org/%242?a=commitdiff_plain;h=0acc8d683ef52e253b9431e74ae9b8edcea8c86c;p=lhc%2Fweb%2Fwiklou.git * 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 f335d024d1..b8455bb76e 100644 --- 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 .= '
' . wfMsgHtml( 'filerevert-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 .= '

' . Xml::inputLabel( wfMsg( 'filerevert-comment' ), 'wpComment', 'wpComment', 60, wfMsgForContent( 'filerevert-defaultcomment', $wgContLang->date( $timestamp, false, false ), $wgContLang->time( $timestamp, false, false ) ) ) . '

'; @@ -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 4906e795fe..5aebb6d050 100644 --- 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;