From: Aryeh Gregor Date: Mon, 29 Apr 2019 14:32:22 +0000 (+0300) Subject: Don't require Title for getTimestampFromId X-Git-Tag: 1.34.0-rc.0~1812 X-Git-Url: http://git.cyclocoop.org/%7B%24admin_url%7Dmes_infos.php?a=commitdiff_plain;h=908e46028aeac08379da82648e4d4a7198445c76;p=lhc%2Fweb%2Fwiklou.git Don't require Title for getTimestampFromId 3e36ba655e3a added an option for passing a page ID to this method of Revision, and e03787afd91c switched it to a Title and made it mandatory. This behavior propagated to the method in RevisionStore. As far as I can tell, the parameter does not help anything, but it can add a database query to get the page ID if it's not cached, and impedes conversion to LinkTarget. I can't figure out any reason to not completely drop it. I've noted it as deprecated but still supported it for now for compatibility -- I found one extension that does pass it. (It's ignored, though, which theoretically would be a behavior change if someone was passing a Title that didn't match the revision ID.) While I was at it, I added the method to RevisionLookup, although it's only used in later patches. Properly I should move that piece to a later patch, but it didn't seem worth the effort. I didn't change the Revision method, because the whole Revision class is deprecated anyway. Change-Id: I26ef5f2bf828f0f450633b7237d26d888e2c8773 --- diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34 index 25888ccbd5..46e85faebb 100644 --- a/RELEASE-NOTES-1.34 +++ b/RELEASE-NOTES-1.34 @@ -25,6 +25,7 @@ Some specific notes for MediaWiki 1.34 upgrades are below: For notes on 1.33.x and older releases, see HISTORY. === Configuration changes for system administrators in 1.34 === + ==== New configuration ==== * … @@ -41,6 +42,7 @@ For notes on 1.33.x and older releases, see HISTORY. * … === External library changes in 1.34 === + ==== New external libraries ==== * … @@ -127,6 +129,8 @@ because of Phabricator reports. * The Config argument to ChangesListSpecialPage::checkStructuredFilterUiEnabled is deprecated. Pass only the User argument. * WatchedItem::getUser is deprecated. Use getUserIdentity. +* Passing a Title as the first parameter to the getTimestampById method of + RevisionStore is deprecated. Omit it, passing only the remaining parameters. === Other changes in 1.34 === * … diff --git a/includes/Revision.php b/includes/Revision.php index cbaff90d69..2b14a21b09 100644 --- a/includes/Revision.php +++ b/includes/Revision.php @@ -1256,13 +1256,13 @@ class Revision implements IDBAccessObject { /** * Get rev_timestamp from rev_id, without loading the rest of the row * - * @param Title $title + * @param Title $title (ignored since 1.34) * @param int $id * @param int $flags * @return string|bool False if not found */ static function getTimestampFromId( $title, $id, $flags = 0 ) { - return self::getRevisionStore()->getTimestampFromId( $title, $id, $flags ); + return self::getRevisionStore()->getTimestampFromId( $id, $flags ); } /** diff --git a/includes/Revision/RevisionLookup.php b/includes/Revision/RevisionLookup.php index db6c7c30b4..4feb9f5430 100644 --- a/includes/Revision/RevisionLookup.php +++ b/includes/Revision/RevisionLookup.php @@ -103,6 +103,18 @@ interface RevisionLookup extends IDBAccessObject { */ public function getNextRevision( RevisionRecord $rev, Title $title = null ); + /** + * Get rev_timestamp from rev_id, without loading the rest of the row. + * + * MCR migration note: this replaces Revision::getTimestampFromId + * + * @param int $id + * @param int $flags + * @return string|bool False if not found + * @since 1.34 (present earlier in RevisionStore) + */ + public function getTimestampFromId( $id, $flags = 0 ); + /** * Load a revision based on a known page ID and current revision ID from the DB * diff --git a/includes/Revision/RevisionStore.php b/includes/Revision/RevisionStore.php index 0329bd1587..515f07d4c5 100644 --- a/includes/Revision/RevisionStore.php +++ b/includes/Revision/RevisionStore.php @@ -2658,21 +2658,27 @@ class RevisionStore } /** - * Get rev_timestamp from rev_id, without loading the rest of the row + * Get rev_timestamp from rev_id, without loading the rest of the row. + * + * Historically, there was an extra Title parameter that was passed before $id. This is no + * longer needed and is deprecated in 1.34. * * MCR migration note: this replaces Revision::getTimestampFromId * - * @param Title $title * @param int $id * @param int $flags * @return string|bool False if not found */ - public function getTimestampFromId( $title, $id, $flags = 0 ) { + public function getTimestampFromId( $id, $flags = 0 ) { + if ( $id instanceof Title ) { + // Old deprecated calling convention supported for backwards compatibility + $id = $flags; + $flags = func_num_args() > 2 ? func_get_arg( 2 ) : 0; + } $db = $this->getDBConnectionRefForQueryFlags( $flags ); - $conds = [ 'rev_id' => $id ]; - $conds['rev_page'] = $title->getArticleID(); - $timestamp = $db->selectField( 'revision', 'rev_timestamp', $conds, __METHOD__ ); + $timestamp = + $db->selectField( 'revision', 'rev_timestamp', [ 'rev_id' => $id ], __METHOD__ ); return ( $timestamp !== false ) ? wfTimestamp( TS_MW, $timestamp ) : false; } diff --git a/includes/api/ApiSetNotificationTimestamp.php b/includes/api/ApiSetNotificationTimestamp.php index ba4c6e8321..d2bbe7bb88 100644 --- a/includes/api/ApiSetNotificationTimestamp.php +++ b/includes/api/ApiSetNotificationTimestamp.php @@ -77,8 +77,9 @@ class ApiSetNotificationTimestamp extends ApiBase { $titles = $pageSet->getGoodTitles(); $title = reset( $titles ); if ( $title ) { + // XXX $title isn't actually used, can we just get rid of the previous six lines? $timestamp = MediaWikiServices::getInstance()->getRevisionStore() - ->getTimestampFromId( $title, $params['torevid'], IDBAccessObject::READ_LATEST ); + ->getTimestampFromId( $params['torevid'], IDBAccessObject::READ_LATEST ); if ( $timestamp ) { $timestamp = $dbw->timestamp( $timestamp ); } else { diff --git a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php index b183fcab47..3467153a55 100644 --- a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php +++ b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php @@ -1416,10 +1416,7 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase { ->value['revision']; $store = MediaWikiServices::getInstance()->getRevisionStore(); - $result = $store->getTimestampFromId( - $page->getTitle(), - $rev->getId() - ); + $result = $store->getTimestampFromId( $rev->getId() ); $this->assertSame( $rev->getTimestamp(), $result ); } @@ -1434,10 +1431,7 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase { ->value['revision']; $store = MediaWikiServices::getInstance()->getRevisionStore(); - $result = $store->getTimestampFromId( - $page->getTitle(), - $rev->getId() + 1 - ); + $result = $store->getTimestampFromId( $rev->getId() + 1 ); $this->assertFalse( $result ); }