Daniel Kinzler has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/400411 )

Change subject: Improve documentation and method naming on Revision and 
RevisionStore.
......................................................................

Improve documentation and method naming on Revision and RevisionStore.

Change-Id: I3b049acff9313814a4ac448289d1aef88cb7f9df
---
M includes/Revision.php
M includes/Storage/RevisionStore.php
M tests/phpunit/includes/Storage/RevisionStoreDbTest.php
M tests/phpunit/includes/Storage/RevisionStoreTest.php
4 files changed, 42 insertions(+), 16 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/11/400411/2

diff --git a/includes/Revision.php b/includes/Revision.php
index 8f36e88..d6c4caa 100644
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -214,8 +214,9 @@
         *
         * 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
@@ -285,7 +286,8 @@
         * 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
@@ -293,7 +295,6 @@
         * @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 );
        }
@@ -482,6 +483,9 @@
 
        /**
         * Do a batched query to get the parent revision lengths
+        *
+        * @deprecated use RevisionStore::getRevisionSizes instead.
+        *
         * @param IDatabase $db
         * @param array $revIds
         * @return array
@@ -503,6 +507,8 @@
                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;
                        }
@@ -794,7 +800,7 @@
         * @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 44dab13..254ca70 100644
--- a/includes/Storage/RevisionStore.php
+++ b/includes/Storage/RevisionStore.php
@@ -562,9 +562,11 @@
        /**
         * MCR migration note: this replaces Revision::isUnpatrolled
         *
+        * @todo This is overly specific, so move or kill this method.
+        *
         * @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 +955,7 @@
         * @param string $timestamp
         * @return RevisionRecord|null
         */
-       public function getRevisionFromTimestamp( $title, $timestamp ) {
+       public function getRevisionByTimestamp( $title, $timestamp ) {
                return $this->newRevisionFromConds(
                        [
                                'rev_timestamp' => $timestamp,
@@ -1651,6 +1653,22 @@
         * @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
+        *         of the corresponding revision.
+        */
        public function listRevisionSizes( IDatabase $db, array $revIds ) {
                $this->checkDatabaseWikiId( $db );
 
diff --git a/tests/phpunit/includes/Storage/RevisionStoreDbTest.php 
b/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
index 695a6b3..4514e5a 100644
--- a/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
+++ b/tests/phpunit/includes/Storage/RevisionStoreDbTest.php
@@ -297,9 +297,9 @@
        }
 
        /**
-        * @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 */
@@ -307,7 +307,7 @@
 
                $store = MediaWikiServices::getInstance()->getRevisionStore();
                $revisionRecord = $store->getRevisionById( $rev->getId() );
-               $result = $store->isUnpatrolled( $revisionRecord );
+               $result = $store->getRcIdIfUnpatrolled( $revisionRecord );
 
                $this->assertGreaterThan( 0, $result );
                $this->assertSame(
@@ -317,9 +317,9 @@
        }
 
        /**
-        * @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' ) );
@@ -335,7 +335,7 @@
 
                $store = MediaWikiServices::getInstance()->getRevisionStore();
                $revisionRecord = $store->getRevisionById( $rev->getId() );
-               $result = $store->isUnpatrolled( $revisionRecord );
+               $result = $store->getRcIdIfUnpatrolled( $revisionRecord );
 
                $this->assertSame( 0, $result );
        }
@@ -410,9 +410,9 @@
        }
 
        /**
-        * @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...
                // :(
@@ -424,7 +424,7 @@
                $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 18dbc250..4cd4ffa7 100644
--- a/tests/phpunit/includes/Storage/RevisionStoreTest.php
+++ b/tests/phpunit/includes/Storage/RevisionStoreTest.php
@@ -291,4 +291,6 @@
                );
        }
 
+       // FIXME: test getRevisionSizes and listRevisionSizes
+
 }

-- 
To view, visit https://gerrit.wikimedia.org/r/400411
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3b049acff9313814a4ac448289d1aef88cb7f9df
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to