From b7204566d2e6b3d7631d0e304b3282c49a5b500d Mon Sep 17 00:00:00 2001 From: Piotr Miazga Date: Wed, 21 Aug 2019 14:10:09 +0200 Subject: [PATCH] Deprecate skin functions that are not skin responsibilities Skin shouldn't be responsible for providing requested revisionId nor if that revision is the current revision. The OutputPage object has all required information (both the currentRevisionID and the current Title object). Change-Id: I2dbae4c6968a2b3b3cea3e09977e9579609b4cc5 --- RELEASE-NOTES-1.34 | 2 ++ includes/OutputPage.php | 10 ++++++++ includes/skins/Skin.php | 16 +++++++------ includes/skins/SkinTemplate.php | 6 ++--- tests/phpunit/includes/OutputPageTest.php | 29 +++++++++++++++++++++++ 5 files changed, 53 insertions(+), 10 deletions(-) diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34 index 8730484a78..df25d30c59 100644 --- a/RELEASE-NOTES-1.34 +++ b/RELEASE-NOTES-1.34 @@ -431,6 +431,8 @@ because of Phabricator reports. engines. * Skin::escapeSearchLink() is deprecated. Use Skin::getSearchLink() or the skin template option 'searchaction' instead. +* Skin::getRevisionId() and Skin::isRevisionCurrent() have been deprecated. + Use OutputPage::getRevisionId() and OutputPage::isRevisionCurrent() instead. * LoadBalancer::haveIndex() and LoadBalancer::isNonZeroLoad() have been deprecated. * FileBackend::getWikiId() has been deprecated. diff --git a/includes/OutputPage.php b/includes/OutputPage.php index f8b7502710..3f2dcf7f4b 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -1661,6 +1661,16 @@ class OutputPage extends ContextSource { return $this->mRevisionId; } + /** + * Whether the revision displayed is the latest revision of the page + * + * @since 1.34 + * @return bool + */ + public function isRevisionCurrent() { + return $this->mRevisionId == 0 || $this->mRevisionId == $this->getTitle()->getLatestRevID(); + } + /** * Set the timestamp of the revision which will be displayed. This is used * to avoid a extra DB call in Skin::lastModified(). diff --git a/includes/skins/Skin.php b/includes/skins/Skin.php index e46f99d14e..bcc257adfd 100644 --- a/includes/skins/Skin.php +++ b/includes/skins/Skin.php @@ -308,6 +308,7 @@ abstract class Skin extends ContextSource { /** * Get the current revision ID * + * @deprecated since 1.34, use OutputPage::getRevisionId instead * @return int */ public function getRevisionId() { @@ -317,11 +318,11 @@ abstract class Skin extends ContextSource { /** * Whether the revision displayed is the latest revision of the page * + * @deprecated since 1.34, use OutputPage::isRevisionCurrent instead * @return bool */ public function isRevisionCurrent() { - $revID = $this->getRevisionId(); - return $revID == 0 || $revID == $this->getTitle()->getLatestRevID(); + return $this->getOutput()->isRevisionCurrent(); } /** @@ -701,7 +702,7 @@ abstract class Skin extends ContextSource { * @return string HTML text with an URL */ function printSource() { - $oldid = $this->getRevisionId(); + $oldid = $this->getOutput()->getRevisionId(); if ( $oldid ) { $canonicalUrl = $this->getTitle()->getCanonicalURL( 'oldid=' . $oldid ); $url = htmlspecialchars( wfExpandIRI( $canonicalUrl ) ); @@ -830,7 +831,7 @@ abstract class Skin extends ContextSource { function getCopyright( $type = 'detect' ) { $linkRenderer = MediaWikiServices::getInstance()->getLinkRenderer(); if ( $type == 'detect' ) { - if ( !$this->isRevisionCurrent() + if ( !$this->getOutput()->isRevisionCurrent() && !$this->msg( 'history_copyright' )->inContentLanguage()->isDisabled() ) { $type = 'history'; @@ -934,7 +935,8 @@ abstract class Skin extends ContextSource { # No cached timestamp, load it from the database if ( $timestamp === null ) { - $timestamp = Revision::getTimestampFromId( $this->getTitle(), $this->getRevisionId() ); + $timestamp = Revision::getTimestampFromId( $this->getTitle(), + $this->getOutput()->getRevisionId() ); } if ( $timestamp ) { @@ -1088,8 +1090,8 @@ abstract class Skin extends ContextSource { function editUrlOptions() { $options = [ 'action' => 'edit' ]; - if ( !$this->isRevisionCurrent() ) { - $options['oldid'] = intval( $this->getRevisionId() ); + if ( !$this->getOutput()->isRevisionCurrent() ) { + $options['oldid'] = intval( $this->getOutput()->getRevisionId() ); } return $options; diff --git a/includes/skins/SkinTemplate.php b/includes/skins/SkinTemplate.php index d1345b8e64..f348135427 100644 --- a/includes/skins/SkinTemplate.php +++ b/includes/skins/SkinTemplate.php @@ -371,7 +371,7 @@ class SkinTemplate extends Skin { $tpl->set( 'credits', false ); $tpl->set( 'numberofwatchingusers', false ); if ( $title->exists() ) { - if ( $out->isArticle() && $this->isRevisionCurrent() ) { + if ( $out->isArticle() && $out->isRevisionCurrent() ) { if ( $wgMaxCredits != 0 ) { /** @var CreditsAction $action */ $action = Action::factory( @@ -975,7 +975,7 @@ class SkinTemplate extends Skin { // Whether to show the "Add a new section" tab // Checks if this is a current rev of talk page and is not forced to be hidden $showNewSection = !$out->forceHideNewSectionLink() - && ( ( $isTalk && $this->isRevisionCurrent() ) || $out->showNewSectionLink() ); + && ( ( $isTalk && $out->isRevisionCurrent() ) || $out->showNewSectionLink() ); $section = $request->getVal( 'section' ); if ( $title->exists() @@ -1295,7 +1295,7 @@ class SkinTemplate extends Skin { if ( $out->isArticle() ) { // Also add a "permalink" while we're at it - $revid = $this->getRevisionId(); + $revid = $this->getOutput()->getRevisionId(); if ( $revid ) { $nav_urls['permalink'] = [ 'text' => $this->msg( 'permalink' )->text(), diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php index 00b8d1823f..6520fc5633 100644 --- a/tests/phpunit/includes/OutputPageTest.php +++ b/tests/phpunit/includes/OutputPageTest.php @@ -3029,6 +3029,35 @@ class OutputPageTest extends MediaWikiTestCase { ]; } + /** + * @param int $titleLastRevision Last Title revision to set + * @param int $outputRevision Revision stored in OutputPage + * @param bool $expectedResult Expected result of $output->isRevisionCurrent call + * @covers OutputPage::isRevisionCurrent + * @dataProvider provideIsRevisionCurrent + */ + public function testIsRevisionCurrent( $titleLastRevision, $outputRevision, $expectedResult ) { + $titleMock = $this->getMock( Title::class, [], [], '', false ); + $titleMock->expects( $this->any() ) + ->method( 'getLatestRevID' ) + ->willReturn( $titleLastRevision ); + + $output = $this->newInstance( [], null, [ 'notitle' => true ] ); + $output->setTitle( $titleMock ); + $output->setRevisionId( $outputRevision ); + $this->assertEquals( $expectedResult, $output->isRevisionCurrent() ); + } + + public function provideIsRevisionCurrent() { + return [ + [ 10, null, true ], + [ 42, 42, true ], + [ null, 0, true ], + [ 42, 47, false ], + [ 47, 42, false ] + ]; + } + /** * @return OutputPage */ -- 2.20.1