FileRepo: Use Late Static Binding in File static constructors

The FileRepo extension API allows to specify factory methods that
FileRepo implementations will use to instantiate File instances.
Currently, the default static constructors in LocalFile and OldLocalFile
do not use Late Static Binding, so every subclass is forced to re-implement them,
even if they would not need any custom logic. Switching to Late Static Binding
(available since PHP 5.3) allows File implementors to reduce boilerplate
if they do not need to overwrite the existing logic.

Change-Id: Id8f6f5362b68269c2a3232796a1703be14116dd5

diff --git a/includes/filerepo/file/ForeignDBFile.php b/includes/filerepo/file/ForeignDBFile.php
index e083a4e..7edefd5 100644 (file)
--- a/includes/filerepo/file/ForeignDBFile.php
+++ b/includes/filerepo/file/ForeignDBFile.php
@@ -31,31 +31,6 @@ use Wikimedia\Rdbms\DBUnexpectedError;
  * @ingroup FileAbstraction
  */
 class ForeignDBFile extends LocalFile {
-       /**
-        * @param Title $title
-        * @param FileRepo $repo
-        * @param null $unused
-        * @return ForeignDBFile
-        */
-       static function newFromTitle( $title, $repo, $unused = null ) {
-               return new self( $title, $repo );
-       }
-
-       /**
-        * Create a ForeignDBFile from a title
-        * Do not call this except from inside a repo class.
-        *
-        * @param stdClass $row
-        * @param FileRepo $repo
-        * @return ForeignDBFile
-        */
-       static function newFromRow( $row, $repo ) {
-               $title = Title::makeTitle( NS_FILE, $row->img_name );
-               $file = new self( $title, $repo );
-               $file->loadFromRow( $row );
-
-               return $file;
-       }
 
        /**
         * @param string $srcPath

diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php
index 86b8bbb..2359fd6 100644 (file)
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -141,10 +141,10 @@ class LocalFile extends File {
         * @param FileRepo $repo
         * @param null $unused
         *
-        * @return self
+        * @return static
         */
        static function newFromTitle( $title, $repo, $unused = null ) {
-               return new self( $title, $repo );
+               return new static( $title, $repo );
        }
 
        /**
@@ -154,11 +154,11 @@ class LocalFile extends File {
         * @param stdClass $row
         * @param FileRepo $repo
         *
-        * @return self
+        * @return static
         */
        static function newFromRow( $row, $repo ) {
                $title = Title::makeTitle( NS_FILE, $row->img_name );
-               $file = new self( $title, $repo );
+               $file = new static( $title, $repo );
                $file->loadFromRow( $row );
 
                return $file;
@@ -181,12 +181,12 @@ class LocalFile extends File {
                        $conds['img_timestamp'] = $dbr->timestamp( $timestamp );
                }
 
-               $fileQuery = self::getQueryInfo();
+               $fileQuery = static::getQueryInfo();
                $row = $dbr->selectRow(
                        $fileQuery['tables'], $fileQuery['fields'], $conds, __METHOD__, [], $fileQuery['joins']
                );
                if ( $row ) {
-                       return self::newFromRow( $row, $repo );
+                       return static::newFromRow( $row, $repo );
                } else {
                        return false;
                }

diff --git a/includes/filerepo/file/OldLocalFile.php b/includes/filerepo/file/OldLocalFile.php
index 3cdbfc2..584e001 100644 (file)
--- a/includes/filerepo/file/OldLocalFile.php
+++ b/includes/filerepo/file/OldLocalFile.php
@@ -42,7 +42,7 @@ class OldLocalFile extends LocalFile {
         * @param Title $title
         * @param FileRepo $repo
         * @param string|int|null $time
-        * @return self
+        * @return static
         * @throws MWException
         */
        static function newFromTitle( $title, $repo, $time = null ) {
@@ -51,27 +51,27 @@ class OldLocalFile extends LocalFile {
                        throw new MWException( __METHOD__ . ' got null for $time parameter' );
                }
 
-               return new self( $title, $repo, $time, null );
+               return new static( $title, $repo, $time, null );
        }
 
        /**
         * @param Title $title
         * @param FileRepo $repo
         * @param string $archiveName
-        * @return self
+        * @return static
         */
        static function newFromArchiveName( $title, $repo, $archiveName ) {
-               return new self( $title, $repo, null, $archiveName );
+               return new static( $title, $repo, null, $archiveName );
        }
 
        /**
         * @param stdClass $row
         * @param FileRepo $repo
-        * @return self
+        * @return static
         */
        static function newFromRow( $row, $repo ) {
                $title = Title::makeTitle( NS_FILE, $row->oi_name );
-               $file = new self( $title, $repo, null, $row->oi_archive_name );
+               $file = new static( $title, $repo, null, $row->oi_archive_name );
                $file->loadFromRow( $row, 'oi_' );
 
                return $file;
@@ -95,12 +95,12 @@ class OldLocalFile extends LocalFile {
                        $conds['oi_timestamp'] = $dbr->timestamp( $timestamp );
                }
 
-               $fileQuery = self::getQueryInfo();
+               $fileQuery = static::getQueryInfo();
                $row = $dbr->selectRow(
                        $fileQuery['tables'], $fileQuery['fields'], $conds, __METHOD__, [], $fileQuery['joins']
                );
                if ( $row ) {
-                       return self::newFromRow( $row, $repo );
+                       return static::newFromRow( $row, $repo );
                } else {
                        return false;
                }

diff --git a/includes/filerepo/file/UnregisteredLocalFile.php b/includes/filerepo/file/UnregisteredLocalFile.php
index fde68bb..2865ce5 100644 (file)
--- a/includes/filerepo/file/UnregisteredLocalFile.php
+++ b/includes/filerepo/file/UnregisteredLocalFile.php
@@ -55,19 +55,19 @@ class UnregisteredLocalFile extends File {
        /**
         * @param string $path Storage path
         * @param string $mime
-        * @return UnregisteredLocalFile
+        * @return static
         */
        static function newFromPath( $path, $mime ) {
-               return new self( false, false, $path, $mime );
+               return new static( false, false, $path, $mime );
        }
 
        /**
         * @param Title $title
         * @param FileRepo $repo
-        * @return UnregisteredLocalFile
+        * @return static
         */
        static function newFromTitle( $title, $repo ) {
-               return new self( $title, $repo, false, false );
+               return new static( $title, $repo, false, false );
        }
 
        /**

diff --git a/tests/phpunit/includes/filerepo/file/ForeignDBFileTest.php b/tests/phpunit/includes/filerepo/file/ForeignDBFileTest.php
new file mode 100644 (file)
index 0000000..3c92ecb
--- /dev/null
+++ b/tests/phpunit/includes/filerepo/file/ForeignDBFileTest.php
@@ -0,0 +1,16 @@
+<?php
+
+/** @covers ForeignDBFile */
+class ForeignDBFileTest extends \PHPUnit\Framework\TestCase {
+
+       use PHPUnit4And6Compat;
+
+       public function testShouldConstructCorrectInstanceFromTitle() {
+               $title = Title::makeTitle( NS_FILE, 'Awesome_file' );
+               $repoMock = $this->createMock( LocalRepo::class );
+
+               $file = ForeignDBFile::newFromTitle( $title, $repoMock );
+
+               $this->assertInstanceOf( ForeignDBFile::class, $file );
+       }
+}