From: Aaron Schulz Date: Wed, 19 Oct 2011 04:06:16 +0000 (+0000) Subject: * Removed newFileFromKey() which had a misleading description and a bug where giving... X-Git-Tag: 1.31.0-rc.0~27016 X-Git-Url: http://git.cyclocoop.org//%22javascript:ModifierStyle%28%27%22.%24id.%22%27%29/%22?a=commitdiff_plain;h=3630c4ee77320af1d360c575c62da8de96c61c59;p=lhc%2Fweb%2Fwiklou.git * Removed newFileFromKey() which had a misleading description and a bug where giving a sha1,time could fail if two current version files existed with the same sha1 but different timestamps. Merged the code into findFileFromKey(). * Fixed bug in findFileFromKey() were it would bail out if it couldn't find a matching current file without even check for matching old files. * Fixed timestamp format of oi_timestamp condition in newFromKey(). * Some code style cleanups. --- diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php index e1ded9e536..2d7a50fb07 100644 --- a/includes/filerepo/FileRepo.php +++ b/includes/filerepo/FileRepo.php @@ -74,7 +74,7 @@ abstract class FileRepo { * @return File */ function newFile( $title, $time = false ) { - if ( !($title instanceof Title) ) { + if ( !( $title instanceof Title ) ) { $title = Title::makeTitleSafe( NS_FILE, $title ); if ( !is_object( $title ) ) { return null; @@ -130,9 +130,9 @@ abstract class FileRepo { if ( $time !== false ) { $img = $this->newFile( $title, $time ); if ( $img && $img->exists() ) { - if ( !$img->isDeleted(File::DELETED_FILE) ) { - return $img; - } elseif ( !empty( $options['private'] ) && $img->userCan(File::DELETED_FILE) ) { + if ( !$img->isDeleted( File::DELETED_FILE ) ) { + return $img; // always OK + } elseif ( !empty( $options['private'] ) && $img->userCan( File::DELETED_FILE ) ) { return $img; } } @@ -186,30 +186,6 @@ abstract class FileRepo { return $result; } - /** - * Create a new File object from the local repository - * @param $sha1 Mixed: base 36 SHA-1 hash - * @param $time Mixed: time at which the image was uploaded. - * If this is specified, the returned object will be an - * of the repository's old file class instead of a current - * file. Repositories not supporting version control should - * return false if this parameter is set. - * - * @return File - */ - function newFileFromKey( $sha1, $time = false ) { - if ( $time ) { - if ( $this->oldFileFactoryKey ) { - return call_user_func( $this->oldFileFactoryKey, $sha1, $this, $time ); - } - } else { - if ( $this->fileFactoryKey ) { - return call_user_func( $this->fileFactoryKey, $sha1, $this ); - } - } - return false; - } - /** * Find an instance of the file with this key, created at the specified time * Returns false if the file does not exist. Repositories not supporting @@ -221,21 +197,22 @@ abstract class FileRepo { function findFileFromKey( $sha1, $options = array() ) { $time = isset( $options['time'] ) ? $options['time'] : false; - # First try the current version of the file to see if it precedes the timestamp - $img = $this->newFileFromKey( $sha1 ); - if ( !$img ) { - return false; + # First try to find a matching current version of a file... + if ( $this->fileFactoryKey ) { + $img = call_user_func( $this->fileFactoryKey, $sha1, $this, $time ); + } else { + return false; // find-by-sha1 not supported } - if ( $img->exists() && ( !$time || $img->getTimestamp() == $time ) ) { + if ( $img && $img->exists() ) { return $img; } - # Now try an old version of the file - if ( $time !== false ) { - $img = $this->newFileFromKey( $sha1, $time ); + # Now try to find a matching old version of a file... + if ( $time !== false && $this->oldFileFactoryKey ) { // find-by-sha1 supported? + $img = call_user_func( $this->oldFileFactoryKey, $sha1, $this, $time ); if ( $img && $img->exists() ) { - if ( !$img->isDeleted(File::DELETED_FILE) ) { - return $img; - } elseif ( !empty( $options['private'] ) && $img->userCan(File::DELETED_FILE) ) { + if ( !$img->isDeleted( File::DELETED_FILE ) ) { + return $img; // always OK + } elseif ( !empty( $options['private'] ) && $img->userCan( File::DELETED_FILE ) ) { return $img; } } diff --git a/includes/filerepo/LocalFile.php b/includes/filerepo/LocalFile.php index 17f7bd604b..70ce9f061a 100644 --- a/includes/filerepo/LocalFile.php +++ b/includes/filerepo/LocalFile.php @@ -95,22 +95,21 @@ class LocalFile extends File { * Create a LocalFile from a SHA-1 key * Do not call this except from inside a repo class. * - * @param $sha1 string + * @param $sha1 string base-36 SHA-1 * @param $repo LocalRepo - * @param $timestamp string + * @param string|bool $timestamp MW_timestamp (optional) * - * @return LocalFile + * @return bool|LocalFile */ static function newFromKey( $sha1, $repo, $timestamp = false ) { - $conds = array( 'img_sha1' => $sha1 ); + $dbr = $repo->getSlaveDB(); + $conds = array( 'img_sha1' => $sha1 ); if ( $timestamp ) { - $conds['img_timestamp'] = $timestamp; + $conds['img_timestamp'] = $dbr->timestamp( $timestamp ); } - $dbr = $repo->getSlaveDB(); $row = $dbr->selectRow( 'image', self::selectFields(), $conds, __METHOD__ ); - if ( $row ) { return self::newFromRow( $row, $repo ); } else { diff --git a/includes/filerepo/OldLocalFile.php b/includes/filerepo/OldLocalFile.php index de168b31f8..a22da16fb1 100644 --- a/includes/filerepo/OldLocalFile.php +++ b/includes/filerepo/OldLocalFile.php @@ -19,8 +19,9 @@ class OldLocalFile extends LocalFile { static function newFromTitle( $title, $repo, $time = null ) { # The null default value is only here to avoid an E_STRICT - if( $time === null ) + if ( $time === null ) { throw new MWException( __METHOD__.' got null for $time parameter' ); + } return new self( $title, $repo, $time, null ); } @@ -36,19 +37,25 @@ class OldLocalFile extends LocalFile { } /** - * @param $sha1 + * Create a OldLocalFile from a SHA-1 key + * Do not call this except from inside a repo class. + * + * @param $sha1 string base-36 SHA-1 * @param $repo LocalRepo - * @param bool $timestamp + * @param string|bool $timestamp MW_timestamp (optional) + * * @return bool|OldLocalFile */ static function newFromKey( $sha1, $repo, $timestamp = false ) { + $dbr = $repo->getSlaveDB(); + $conds = array( 'oi_sha1' => $sha1 ); - if( $timestamp ) { - $conds['oi_timestamp'] = $timestamp; + if ( $timestamp ) { + $conds['oi_timestamp'] = $dbr->timestamp( $timestamp ); } - $dbr = $repo->getSlaveDB(); + $row = $dbr->selectRow( 'oldimage', self::selectFields(), $conds, __METHOD__ ); - if( $row ) { + if ( $row ) { return self::newFromRow( $row, $repo ); } else { return false;