Merge "Use native ES5 Array prototype methods instead of jQuery"

diff --git a/includes/Revision.php b/includes/Revision.php
index 66b4173..510c1ee 100644 (file)
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -495,13 +495,13 @@ class Revision implements IDBAccessObject {
                        $this->mRecord = self::getRevisionStore()->newMutableRevisionFromArray(
                                $row,
                                $queryFlags,
-                               $title
+                               $this->ensureTitle( $row, $queryFlags, $title )
                        );
                } elseif ( is_object( $row ) ) {
                        $this->mRecord = self::getRevisionStore()->newRevisionFromRow(
                                $row,
                                $queryFlags,
-                               $title
+                               $this->ensureTitle( $row, $queryFlags, $title )
                        );
                } else {
                        throw new InvalidArgumentException(
@@ -510,6 +510,51 @@ class Revision implements IDBAccessObject {
                }
        }
 
+       /**
+        * Make sure we have *some* Title object for use by the constructor.
+        * For B/C, the constructor shouldn't fail even for a bad page ID or bad revision ID.
+        *
+        * @param array|object $row
+        * @param int $queryFlags
+        * @param Title|null $title
+        *
+        * @return Title $title if not null, or a Title constructed from information in $row.
+        */
+       private function ensureTitle( $row, $queryFlags, $title = null ) {
+               if ( $title ) {
+                       return $title;
+               }
+
+               if ( is_array( $row ) ) {
+                       if ( isset( $row['title'] ) ) {
+                               if ( !( $row['title'] instanceof Title ) ) {
+                                       throw new MWException( 'title field must contain a Title object.' );
+                               }
+
+                               return $row['title'];
+                       }
+
+                       $pageId = isset( $row['page'] ) ? $row['page'] : 0;
+                       $revId = isset( $row['id'] ) ? $row['id'] : 0;
+               } else {
+                       $pageId = isset( $row->rev_page ) ? $row->rev_page : 0;
+                       $revId = isset( $row->rev_id ) ? $row->rev_id : 0;
+               }
+
+               try {
+                       $title = self::getRevisionStore()->getTitle( $pageId, $revId, $queryFlags );
+               } catch ( RevisionAccessException $ex ) {
+                       // construct a dummy title!
+                       wfLogWarning( __METHOD__ . ': ' . $ex->getMessage() );
+
+                       // NOTE: this Title will only be used inside RevisionRecord
+                       $title = Title::makeTitleSafe( NS_SPECIAL, "Badtitle/ID=$pageId" );
+                       $title->resetArticleID( $pageId );
+               }
+
+               return $title;
+       }
+
        /**
         * @return RevisionRecord
         */

diff --git a/includes/Storage/RevisionStore.php b/includes/Storage/RevisionStore.php
index 78e789e..6c8cac9 100644 (file)
--- a/includes/Storage/RevisionStore.php
+++ b/includes/Storage/RevisionStore.php
@@ -164,6 +164,8 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup
         *
         * MCR migration note: this corresponds to Revision::getTitle
         *
+        * @note this method should be private, external use should be avoided!
+        *
         * @param int|null $pageId
         * @param int|null $revId
         * @param int $queryFlags
@@ -171,7 +173,7 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup
         * @return Title
         * @throws RevisionAccessException
         */
-       private function getTitle( $pageId, $revId, $queryFlags = 0 ) {
+       public function getTitle( $pageId, $revId, $queryFlags = 0 ) {
                if ( !$pageId && !$revId ) {
                        throw new InvalidArgumentException( '$pageId and $revId cannot both be 0 or null' );
                }

diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index 83d2ae9..4d7ef28 100644 (file)
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -155,6 +155,7 @@ abstract class ApiBase extends ContextSource {
         * ((string|array|Message)[]) When PARAM_TYPE is an array, this is an array
         * mapping those values to $msg for ApiBase::makeMessage(). Any value not
         * having a mapping will use apihelp-{$path}-paramvalue-{$param}-{$value}.
+        * Specify an empty array to use the default message key for all values.
         * @since 1.25
         */
        const PARAM_HELP_MSG_PER_VALUE = 14;

diff --git a/tests/phpunit/includes/RevisionTest.php b/tests/phpunit/includes/RevisionTest.php
index a467346..73d69a5 100644 (file)
--- a/tests/phpunit/includes/RevisionTest.php
+++ b/tests/phpunit/includes/RevisionTest.php
@@ -80,6 +80,17 @@ class RevisionTest extends MediaWikiTestCase {
                $this->assertNull( $rev->getContent(), 'no content object should be available' );
        }
 
+       /**
+        * @covers Revision::__construct
+        * @covers \MediaWiki\Storage\RevisionStore::newMutableRevisionFromArray
+        */
+       public function testConstructFromArrayWithBadPageId() {
+               MediaWiki\suppressWarnings();
+               $rev = new Revision( [ 'page' => 77777777 ] );
+               $this->assertSame( 77777777, $rev->getPage() );
+               MediaWiki\restoreWarnings();
+       }
+
        public function provideConstructFromArray_userSetAsExpected() {
                yield 'no user defaults to wgUser' => [
                        [
@@ -301,6 +312,17 @@ class RevisionTest extends MediaWikiTestCase {
                $assertions( $this, $rev );
        }
 
+       /**
+        * @covers Revision::__construct
+        * @covers \MediaWiki\Storage\RevisionStore::newMutableRevisionFromArray
+        */
+       public function testConstructFromRowWithBadPageId() {
+               MediaWiki\suppressWarnings();
+               $rev = new Revision( (object)[ 'rev_page' => 77777777 ] );
+               $this->assertSame( 77777777, $rev->getPage() );
+               MediaWiki\restoreWarnings();
+       }
+
        public function provideGetRevisionText() {
                yield 'Generic test' => [
                        'This is a goat of revision text.',

diff --git a/tests/phpunit/includes/Storage/RevisionStoreTest.php b/tests/phpunit/includes/Storage/RevisionStoreTest.php
index 50c0884..2cdd316 100644 (file)
--- a/tests/phpunit/includes/Storage/RevisionStoreTest.php
+++ b/tests/phpunit/includes/Storage/RevisionStoreTest.php
@@ -2,10 +2,12 @@
 
 namespace MediaWiki\Tests\Storage;
 
+use MediaWiki\Storage\RevisionAccessException;
 use MediaWiki\Storage\RevisionStore;
 use MediaWiki\Storage\SqlBlobStore;
 use MediaWikiTestCase;
 use WANObjectCache;
+use Wikimedia\Rdbms\Database;
 use Wikimedia\Rdbms\LoadBalancer;
 
 class RevisionStoreTest extends MediaWikiTestCase {
@@ -37,6 +39,14 @@ class RevisionStoreTest extends MediaWikiTestCase {
                        ->disableOriginalConstructor()->getMock();
        }
 
+       /**
+        * @return \PHPUnit_Framework_MockObject_MockObject|Database
+        */
+       private function getMockDatabase() {
+               return $this->getMockBuilder( Database::class )
+                       ->disableOriginalConstructor()->getMock();
+       }
+
        /**
         * @return \PHPUnit_Framework_MockObject_MockObject|SqlBlobStore
         */
@@ -291,6 +301,126 @@ class RevisionStoreTest extends MediaWikiTestCase {
                );
        }
 
+       public function testGetTitle_successFromPageId() {
+               $mockLoadBalancer = $this->getMockLoadBalancer();
+               // Title calls wfGetDB() so we have to set the main service
+               $this->setService( 'DBLoadBalancer', $mockLoadBalancer );
+
+               $db = $this->getMockDatabase();
+               // Title calls wfGetDB() which uses a regular Connection
+               $mockLoadBalancer->expects( $this->atLeastOnce() )
+                       ->method( 'getConnection' )
+                       ->willReturn( $db );
+
+               // First call to Title::newFromID, faking no result (db lag?)
+               $db->expects( $this->at( 0 ) )
+                       ->method( 'selectRow' )
+                       ->with(
+                               'page',
+                               $this->anything(),
+                               [ 'page_id' => 1 ]
+                       )
+                       ->willReturn( (object)[
+                               'page_namespace' => '1',
+                               'page_title' => 'Food',
+                       ] );
+
+               $store = $this->getRevisionStore( $mockLoadBalancer );
+               $title = $store->getTitle( 1, 2, RevisionStore::READ_NORMAL );
+
+               $this->assertSame( 1, $title->getNamespace() );
+               $this->assertSame( 'Food', $title->getDBkey() );
+       }
+
+       public function testGetTitle_successFromRevId() {
+               $mockLoadBalancer = $this->getMockLoadBalancer();
+               // Title calls wfGetDB() so we have to set the main service
+               $this->setService( 'DBLoadBalancer', $mockLoadBalancer );
+
+               $db = $this->getMockDatabase();
+               // Title calls wfGetDB() which uses a regular Connection
+               $mockLoadBalancer->expects( $this->atLeastOnce() )
+                       ->method( 'getConnection' )
+                       ->willReturn( $db );
+               // RevisionStore getTitle uses a ConnectionRef
+               $mockLoadBalancer->expects( $this->atLeastOnce() )
+                       ->method( 'getConnectionRef' )
+                       ->willReturn( $db );
+
+               // First call to Title::newFromID, faking no result (db lag?)
+               $db->expects( $this->at( 0 ) )
+                       ->method( 'selectRow' )
+                       ->with(
+                               'page',
+                               $this->anything(),
+                               [ 'page_id' => 1 ]
+                       )
+                       ->willReturn( false );
+
+               // First select using rev_id, faking no result (db lag?)
+               $db->expects( $this->at( 1 ) )
+                       ->method( 'selectRow' )
+                       ->with(
+                               [ 'revision', 'page' ],
+                               $this->anything(),
+                               [ 'rev_id' => 2 ]
+                       )
+                       ->willReturn( (object)[
+                               'page_namespace' => '1',
+                               'page_title' => 'Food2',
+                       ] );
+
+               $store = $this->getRevisionStore( $mockLoadBalancer );
+               $title = $store->getTitle( 1, 2, RevisionStore::READ_NORMAL );
+
+               $this->assertSame( 1, $title->getNamespace() );
+               $this->assertSame( 'Food2', $title->getDBkey() );
+       }
+
+       /**
+        * @covers \MediaWiki\Storage\RevisionStore::getTitle
+        */
+       public function testGetTitle_throwsExceptionAfterFallbacks() {
+               $mockLoadBalancer = $this->getMockLoadBalancer();
+               // Title calls wfGetDB() so we have to set the main service
+               $this->setService( 'DBLoadBalancer', $mockLoadBalancer );
+
+               $db = $this->getMockDatabase();
+               // Title calls wfGetDB() which uses a regular Connection
+               $mockLoadBalancer->expects( $this->atLeastOnce() )
+                       ->method( 'getConnection' )
+                       ->willReturn( $db );
+               // RevisionStore getTitle uses a ConnectionRef
+               $mockLoadBalancer->expects( $this->atLeastOnce() )
+                       ->method( 'getConnectionRef' )
+                       ->willReturn( $db );
+
+               // First call to Title::newFromID, faking no result (db lag?)
+               $db->expects( $this->at( 0 ) )
+                       ->method( 'selectRow' )
+                       ->with(
+                               'page',
+                               $this->anything(),
+                               [ 'page_id' => 1 ]
+                       )
+                       ->willReturn( false );
+
+               // First select using rev_id, faking no result (db lag?)
+               $db->expects( $this->at( 1 ) )
+                       ->method( 'selectRow' )
+                       ->with(
+                               [ 'revision', 'page' ],
+                               $this->anything(),
+                               [ 'rev_id' => 2 ]
+                       )
+                       ->willReturn( false );
+
+               $store = $this->getRevisionStore( $mockLoadBalancer );
+
+               $this->setExpectedException( RevisionAccessException::class );
+               $store->getTitle( 1, 2, RevisionStore::READ_NORMAL );
+       }
+
        // FIXME: test getRevisionSizes
 
 }