* Removed newFileFromKey() which had a misleading description and a bug where giving...

* 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 e1ded9e..2d7a50f 100644 (file)
--- 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 17f7bd6..70ce9f0 100644 (file)
--- 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 de168b3..a22da16 100644 (file)
--- 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;