[MCR] Improve documentation and method naming on Revision and RevisionStore.

Change-Id: I3b049acff9313814a4ac448289d1aef88cb7f9df

diff --git a/includes/Revision.php b/includes/Revision.php
index b1440d0..10b896b 100644 (file)
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -193,8 +193,9 @@ class Revision implements IDBAccessObject {
         *
         * MCR migration note: replaced by RevisionStore::newRevisionFromRow(). Note that
         * newFromRow() also accepts arrays, while newRevisionFromRow() does not. Instead,
-        * a MutableRevisionRecord should be constructed directly. RevisionStore::newRevisionFromArray()
-        * can be used as a temporary replacement, but should be avoided.
+        * a MutableRevisionRecord should be constructed directly.
+        * RevisionStore::newMutableRevisionFromArray() can be used as a temporary replacement,
+        * but should be avoided.
         *
         * @param object|array $row
         * @return Revision
@@ -264,7 +265,8 @@ class Revision implements IDBAccessObject {
         * WARNING: Timestamps may in some circumstances not be unique,
         * so this isn't the best key to use.
         *
-        * @deprecated since 1.31, use RevisionStore::loadRevisionFromTimestamp() instead.
+        * @deprecated since 1.31, use RevisionStore::getRevisionByTimestamp()
+        *   or RevisionStore::loadRevisionFromTimestamp() instead.
         *
         * @param IDatabase $db
         * @param Title $title
@@ -272,7 +274,6 @@ class Revision implements IDBAccessObject {
         * @return Revision|null
         */
        public static function loadFromTimestamp( $db, $title, $timestamp ) {
-               // XXX: replace loadRevisionFromTimestamp by getRevisionByTimestamp?
                $rec = self::getRevisionStore()->loadRevisionFromTimestamp( $db, $title, $timestamp );
                return $rec === null ? null : new Revision( $rec );
        }
@@ -461,6 +462,9 @@ class Revision implements IDBAccessObject {
 
        /**
         * Do a batched query to get the parent revision lengths
+        *
+        * @deprecated in 1.31, use RevisionStore::getRevisionSizes instead.
+        *
         * @param IDatabase $db
         * @param array $revIds
         * @return array
@@ -482,6 +486,8 @@ class Revision implements IDBAccessObject {
                if ( $row instanceof RevisionRecord ) {
                        $this->mRecord = $row;
                } elseif ( is_array( $row ) ) {
+                       // If no user is specified, fall back to using the global user object, to stay
+                       // compatible with pre-1.31 behavior.
                        if ( !isset( $row['user'] ) && !isset( $row['user_text'] ) ) {
                                $row['user'] = $wgUser;
                        }
@@ -773,7 +779,7 @@ class Revision implements IDBAccessObject {
         * @return int Rcid of the unpatrolled row, zero if there isn't one
         */
        public function isUnpatrolled() {
-               return self::getRevisionStore()->isUnpatrolled( $this->mRecord );
+               return self::getRevisionStore()->getRcIdIfUnpatrolled( $this->mRecord );
        }
 
        /**

diff --git a/includes/Storage/RevisionStore.php b/includes/Storage/RevisionStore.php
index 0950109..3101aea 100644 (file)
--- a/includes/Storage/RevisionStore.php
+++ b/includes/Storage/RevisionStore.php
@@ -562,10 +562,13 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup
        /**
         * MCR migration note: this replaces Revision::isUnpatrolled
         *
+        * @todo This is overly specific, so move or kill this method.
+        *
         * @param RevisionRecord $rev
+        *
         * @return int Rcid of the unpatrolled row, zero if there isn't one
         */
-       public function isUnpatrolled( RevisionRecord $rev ) {
+       public function getRcIdIfUnpatrolled( RevisionRecord $rev ) {
                $rc = $this->getRecentChange( $rev );
                if ( $rc && $rc->getAttribute( 'rc_patrolled' ) == 0 ) {
                        return $rc->getAttribute( 'rc_id' );
@@ -953,7 +956,7 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup
         * @param string $timestamp
         * @return RevisionRecord|null
         */
-       public function getRevisionFromTimestamp( $title, $timestamp ) {
+       public function getRevisionByTimestamp( $title, $timestamp ) {
                return $this->newRevisionFromConds(
                        [
                                'rev_timestamp' => $timestamp,
@@ -1656,6 +1659,21 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup
         *
         * MCR migration note: this replaces Revision::getParentLengths
         *
+        * @param int[] $revIds
+        * @return int[] associative array mapping revision IDs from $revIds to the nominal size
+        *         of the corresponding revision.
+        */
+       public function getRevisionSizes( array $revIds ) {
+               return $this->listRevisionSizes( $this->getDBConnection( DB_REPLICA ), $revIds );
+       }
+
+       /**
+        * Do a batched query for the sizes of a set of revisions.
+        *
+        * MCR migration note: this replaces Revision::getParentLengths
+        *
+        * @deprecated use RevisionStore::getRevisionSizes instead.
+        *
         * @param IDatabase $db
         * @param int[] $revIds
         * @return int[] associative array mapping revision IDs from $revIds to the nominal size

diff --git a/tests/phpunit/includes/Storage/RevisionStoreDbTest.php b/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
index 28a8d88..15f173a 100644 (file)
--- a/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
+++ b/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
@@ -407,9 +407,9 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
        }
 
        /**
-        * @covers \MediaWiki\Storage\RevisionStore::isUnpatrolled
+        * @covers \MediaWiki\Storage\RevisionStore::getRcIdIfUnpatrolled
         */
-       public function testIsUnpatrolled_returnsRecentChangesId() {
+       public function testGetRcIdIfUnpatrolled_returnsRecentChangesId() {
                $page = WikiPage::factory( Title::newFromText( 'UTPage' ) );
                $status = $page->doEditContent( new WikitextContent( __METHOD__ ), __METHOD__ );
                /** @var Revision $rev */
@@ -417,7 +417,7 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
 
                $store = MediaWikiServices::getInstance()->getRevisionStore();
                $revisionRecord = $store->getRevisionById( $rev->getId() );
-               $result = $store->isUnpatrolled( $revisionRecord );
+               $result = $store->getRcIdIfUnpatrolled( $revisionRecord );
 
                $this->assertGreaterThan( 0, $result );
                $this->assertSame(
@@ -427,9 +427,9 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
        }
 
        /**
-        * @covers \MediaWiki\Storage\RevisionStore::isUnpatrolled
+        * @covers \MediaWiki\Storage\RevisionStore::getRcIdIfUnpatrolled
         */
-       public function testIsUnpatrolled_returnsZeroIfPatrolled() {
+       public function testGetRcIdIfUnpatrolled_returnsZeroIfPatrolled() {
                // This assumes that sysops are auto patrolled
                $sysop = $this->getTestSysop()->getUser();
                $page = WikiPage::factory( Title::newFromText( 'UTPage' ) );
@@ -445,7 +445,7 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
 
                $store = MediaWikiServices::getInstance()->getRevisionStore();
                $revisionRecord = $store->getRevisionById( $rev->getId() );
-               $result = $store->isUnpatrolled( $revisionRecord );
+               $result = $store->getRcIdIfUnpatrolled( $revisionRecord );
 
                $this->assertSame( 0, $result );
        }
@@ -523,9 +523,9 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
        }
 
        /**
-        * @covers \MediaWiki\Storage\RevisionStore::getRevisionFromTimestamp
+        * @covers \MediaWiki\Storage\RevisionStore::getRevisionByTimestamp
         */
-       public function testGetRevisionFromTimestamp() {
+       public function testGetRevisionByTimestamp() {
                // Make sure there is 1 second between the last revision and the rev we create...
                // Otherwise we might not get the correct revision and the test may fail...
                // :(
@@ -537,7 +537,7 @@ class RevisionStoreDbTest extends MediaWikiTestCase {
                $rev = $status->value['revision'];
 
                $store = MediaWikiServices::getInstance()->getRevisionStore();
-               $revRecord = $store->getRevisionFromTimestamp(
+               $revRecord = $store->getRevisionByTimestamp(
                        $page->getTitle(),
                        $rev->getTimestamp()
                );

diff --git a/tests/phpunit/includes/Storage/RevisionStoreTest.php b/tests/phpunit/includes/Storage/RevisionStoreTest.php
index 18dbc25..50c0884 100644 (file)
--- a/tests/phpunit/includes/Storage/RevisionStoreTest.php
+++ b/tests/phpunit/includes/Storage/RevisionStoreTest.php
@@ -291,4 +291,6 @@ class RevisionStoreTest extends MediaWikiTestCase {
                );
        }
 
+       // FIXME: test getRevisionSizes
+
 }