From e30cb5ba908609f33bcabd59114065e10dbdbdd2 Mon Sep 17 00:00:00 2001 From: Aryeh Gregor Date: Mon, 29 Apr 2019 19:29:31 +0300 Subject: [PATCH] Move Title::getSubject/Talk/OtherPage to NamespaceInfo This allows converting some more code to LinkTarget. 100% test coverage. Change-Id: I28903af6a41d02755f37f31561a524547445821e --- RELEASE-NOTES-1.34 | 2 + includes/Title.php | 27 +- includes/title/NamespaceInfo.php | 36 +++ tests/phpunit/includes/TitleTest.php | 13 - .../includes/title/NamespaceInfoTest.php | 283 +++++++++++++----- 5 files changed, 251 insertions(+), 110 deletions(-) diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34 index 98ed961605..b58c2694b7 100644 --- a/RELEASE-NOTES-1.34 +++ b/RELEASE-NOTES-1.34 @@ -145,6 +145,8 @@ because of Phabricator reports. * RepoGroup::singleton(), RepoGroup::destroySingleton(), RepoGroup::setSingleton(), wfFindFile(), and wfLocalFile() are all deprecated. Use MediaWikiServices instead. +* The getSubjectPage, getTalkPage, and getOtherPage of Title are deprecated. + Use NamespaceInfo's getSubjectPage, getTalkPage, and getAssociatedPage. === Other changes in 1.34 === * … diff --git a/includes/Title.php b/includes/Title.php index 4cac844294..dc5400e8e4 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -1501,10 +1501,12 @@ class Title implements LinkTarget, IDBAccessObject { /** * Get a Title object associated with the talk page of this article * + * @deprecated since 1.34, use NamespaceInfo::getTalkPage * @return Title The object for the talk page */ public function getTalkPage() { - return self::makeTitle( MWNamespace::getTalk( $this->mNamespace ), $this->mDbkeyform ); + return self::castFromLinkTarget( + MediaWikiServices::getInstance()->getNamespaceInfo()->getTalkPage( $this ) ); } /** @@ -1528,37 +1530,26 @@ class Title implements LinkTarget, IDBAccessObject { * Get a title object associated with the subject page of this * talk page * + * @deprecated since 1.34, use NamespaceInfo::getSubjectPage * @return Title The object for the subject page */ public function getSubjectPage() { - // Is this the same title? - $subjectNS = MWNamespace::getSubject( $this->mNamespace ); - if ( $this->mNamespace == $subjectNS ) { - return $this; - } - return self::makeTitle( $subjectNS, $this->mDbkeyform ); + return self::castFromLinkTarget( + MediaWikiServices::getInstance()->getNamespaceInfo()->getSubjectPage( $this ) ); } /** * Get the other title for this page, if this is a subject page * get the talk page, if it is a subject page get the talk page * + * @deprecated since 1.34, use NamespaceInfo::getAssociatedPage * @since 1.25 * @throws MWException If the page doesn't have an other page * @return Title */ public function getOtherPage() { - if ( $this->isSpecialPage() ) { - throw new MWException( 'Special pages cannot have other pages' ); - } - if ( $this->isTalkPage() ) { - return $this->getSubjectPage(); - } else { - if ( !$this->canHaveTalkPage() ) { - throw new MWException( "{$this->getPrefixedText()} does not have an other page" ); - } - return $this->getTalkPage(); - } + return self::castFromLinkTarget( + MediaWikiServices::getInstance()->getNamespaceInfo()->getAssociatedPage( $this ) ); } /** diff --git a/includes/title/NamespaceInfo.php b/includes/title/NamespaceInfo.php index e3ab3a3aba..7cfadc0144 100644 --- a/includes/title/NamespaceInfo.php +++ b/includes/title/NamespaceInfo.php @@ -21,6 +21,7 @@ */ use MediaWiki\Config\ServiceOptions; +use MediaWiki\Linker\LinkTarget; /** * This is a utility class for dealing with namespaces that encodes all the "magic" behaviors of @@ -149,6 +150,18 @@ class NamespaceInfo { : $index + 1; } + /** + * @param LinkTarget $target + * @return LinkTarget Talk page for $target + * @throws MWException if $target's namespace doesn't have talk pages (e.g., NS_SPECIAL) + */ + public function getTalkPage( LinkTarget $target ) : LinkTarget { + if ( $this->isTalk( $target->getNamespace() ) ) { + return $target; + } + return new TitleValue( $this->getTalk( $target->getNamespace() ), $target->getDbKey() ); + } + /** * Get the subject namespace index for a given namespace * Special namespaces (NS_MEDIA, NS_SPECIAL) are always the subject. @@ -167,6 +180,17 @@ class NamespaceInfo { : $index; } + /** + * @param LinkTarget $target + * @return LinkTarget Subject page for $target + */ + public function getSubjectPage( LinkTarget $target ) : LinkTarget { + if ( $this->isSubject( $target->getNamespace() ) ) { + return $target; + } + return new TitleValue( $this->getSubject( $target->getNamespace() ), $target->getDbKey() ); + } + /** * Get the associated namespace. * For talk namespaces, returns the subject (non-talk) namespace @@ -174,6 +198,7 @@ class NamespaceInfo { * * @param int $index Namespace index * @return int + * @throws MWException if called on a namespace that has no talk pages (e.g., NS_SPECIAL) */ public function getAssociated( $index ) { $this->isMethodValidFor( $index, __METHOD__ ); @@ -184,6 +209,17 @@ class NamespaceInfo { return $this->getSubject( $index ); } + /** + * @param LinkTarget $target + * @return LinkTarget Talk page for $target if it's a subject page, subject page if it's a talk + * page + * @throws MWException if $target's namespace doesn't have talk pages (e.g., NS_SPECIAL) + */ + public function getAssociatedPage( LinkTarget $target ) : LinkTarget { + return new TitleValue( + $this->getAssociated( $target->getNamespace() ), $target->getDbKey() ); + } + /** * Returns whether the specified namespace exists * diff --git a/tests/phpunit/includes/TitleTest.php b/tests/phpunit/includes/TitleTest.php index 2159a35bf3..c46f69b2ef 100644 --- a/tests/phpunit/includes/TitleTest.php +++ b/tests/phpunit/includes/TitleTest.php @@ -786,19 +786,6 @@ class TitleTest extends MediaWikiTestCase { ]; } - /** - * @dataProvider provideGetTalkPage_good - * @covers Title::getTalkPage - */ - public function testGetTalkPage_good( Title $title, Title $expected ) { - $talk = $title->getTalkPage(); - $this->assertSame( - $expected->getPrefixedDBKey(), - $talk->getPrefixedDBKey(), - $title->getPrefixedDBKey() - ); - } - /** * @dataProvider provideGetTalkPage_good * @covers Title::getTalkPageIfDefined diff --git a/tests/phpunit/includes/title/NamespaceInfoTest.php b/tests/phpunit/includes/title/NamespaceInfoTest.php index 5e6763677f..556c640bd6 100644 --- a/tests/phpunit/includes/title/NamespaceInfoTest.php +++ b/tests/phpunit/includes/title/NamespaceInfoTest.php @@ -166,85 +166,6 @@ class NamespaceInfoTest extends MediaWikiTestCase { ]; } - /** - * @covers NamespaceInfo::getSubject - */ - public function testGetSubject() { - // Special namespaces are their own subjects - $obj = $this->newObj(); - $this->assertEquals( NS_MEDIA, $obj->getSubject( NS_MEDIA ) ); - $this->assertEquals( NS_SPECIAL, $obj->getSubject( NS_SPECIAL ) ); - - $this->assertEquals( NS_MAIN, $obj->getSubject( NS_TALK ) ); - $this->assertEquals( NS_USER, $obj->getSubject( NS_USER_TALK ) ); - } - - /** - * Regular getTalk() calls - * Namespaces without a talk page (NS_MEDIA, NS_SPECIAL) are tested in - * the function testGetTalkExceptions() - * @covers NamespaceInfo::getTalk - * @covers NamespaceInfo::isMethodValidFor - */ - public function testGetTalk() { - $obj = $this->newObj(); - $this->assertEquals( NS_TALK, $obj->getTalk( NS_MAIN ) ); - $this->assertEquals( NS_TALK, $obj->getTalk( NS_TALK ) ); - $this->assertEquals( NS_USER_TALK, $obj->getTalk( NS_USER ) ); - $this->assertEquals( NS_USER_TALK, $obj->getTalk( NS_USER_TALK ) ); - } - - /** - * Exceptions with getTalk() - * NS_MEDIA does not have talk pages. MediaWiki raise an exception for them. - * @expectedException MWException - * @covers NamespaceInfo::getTalk - * @covers NamespaceInfo::isMethodValidFor - */ - public function testGetTalkExceptionsForNsMedia() { - $this->assertNull( $this->newObj()->getTalk( NS_MEDIA ) ); - } - - /** - * Exceptions with getTalk() - * NS_SPECIAL does not have talk pages. MediaWiki raise an exception for them. - * @expectedException MWException - * @covers NamespaceInfo::getTalk - */ - public function testGetTalkExceptionsForNsSpecial() { - $this->assertNull( $this->newObj()->getTalk( NS_SPECIAL ) ); - } - - /** - * Regular getAssociated() calls - * Namespaces without an associated page (NS_MEDIA, NS_SPECIAL) are tested in - * the function testGetAssociatedExceptions() - * @covers NamespaceInfo::getAssociated - */ - public function testGetAssociated() { - $this->assertEquals( NS_TALK, $this->newObj()->getAssociated( NS_MAIN ) ); - $this->assertEquals( NS_MAIN, $this->newObj()->getAssociated( NS_TALK ) ); - } - - # ## Exceptions with getAssociated() - # ## NS_MEDIA and NS_SPECIAL do not have talk pages. MediaWiki raises - # ## an exception for them. - /** - * @expectedException MWException - * @covers NamespaceInfo::getAssociated - */ - public function testGetAssociatedExceptionsForNsMedia() { - $this->assertNull( $this->newObj()->getAssociated( NS_MEDIA ) ); - } - - /** - * @expectedException MWException - * @covers NamespaceInfo::getAssociated - */ - public function testGetAssociatedExceptionsForNsSpecial() { - $this->assertNull( $this->newObj()->getAssociated( NS_SPECIAL ) ); - } - /** * @covers NamespaceInfo::exists * @dataProvider provideExists @@ -676,6 +597,210 @@ class NamespaceInfoTest extends MediaWikiTestCase { // %} End basic methods + /********************************************************************************************** + * getSubject/Talk/Associated + * %{ + */ + /** + * @dataProvider provideSubjectTalk + * @covers NamespaceInfo::getSubject + * @covers NamespaceInfo::getSubjectPage + * @covers NamespaceInfo::isMethodValidFor + * @covers Title::getSubjectPage + * + * @param int $subject + * @param int $talk + */ + public function testGetSubject( $subject, $talk ) { + $obj = $this->newObj(); + $this->assertSame( $subject, $obj->getSubject( $subject ) ); + $this->assertSame( $subject, $obj->getSubject( $talk ) ); + + $subjectTitleVal = new TitleValue( $subject, 'A' ); + $talkTitleVal = new TitleValue( $talk, 'A' ); + // Object will be the same one passed in if it's a subject, different but equal object if + // it's talk + $this->assertSame( $subjectTitleVal, $obj->getSubjectPage( $subjectTitleVal ) ); + $this->assertEquals( $subjectTitleVal, $obj->getSubjectPage( $talkTitleVal ) ); + + $subjectTitle = Title::makeTitle( $subject, 'A' ); + $talkTitle = Title::makeTitle( $talk, 'A' ); + $this->assertSame( $subjectTitle, $subjectTitle->getSubjectPage() ); + $this->assertEquals( $subjectTitle, $talkTitle->getSubjectPage() ); + } + + /** + * @dataProvider provideSpecialNamespaces + * @covers NamespaceInfo::getSubject + * @covers NamespaceInfo::getSubjectPage + * + * @param int $ns + */ + public function testGetSubject_special( $ns ) { + $obj = $this->newObj(); + $this->assertSame( $ns, $obj->getSubject( $ns ) ); + + $title = new TitleValue( $ns, 'A' ); + $this->assertSame( $title, $obj->getSubjectPage( $title ) ); + } + + /** + * @dataProvider provideSubjectTalk + * @covers NamespaceInfo::getTalk + * @covers NamespaceInfo::getTalkPage + * @covers NamespaceInfo::isMethodValidFor + * @covers Title::getTalkPage + * + * @param int $subject + * @param int $talk + */ + public function testGetTalk( $subject, $talk ) { + $obj = $this->newObj(); + $this->assertSame( $talk, $obj->getTalk( $subject ) ); + $this->assertSame( $talk, $obj->getTalk( $talk ) ); + + $subjectTitleVal = new TitleValue( $subject, 'A' ); + $talkTitleVal = new TitleValue( $talk, 'A' ); + // Object will be the same one passed in if it's a talk, different but equal object if it's + // subject + $this->assertEquals( $talkTitleVal, $obj->getTalkPage( $subjectTitleVal ) ); + $this->assertSame( $talkTitleVal, $obj->getTalkPage( $talkTitleVal ) ); + + $subjectTitle = Title::makeTitle( $subject, 'A' ); + $talkTitle = Title::makeTitle( $talk, 'A' ); + $this->assertEquals( $talkTitle, $subjectTitle->getTalkPage() ); + $this->assertSame( $talkTitle, $talkTitle->getTalkPage() ); + } + + /** + * @dataProvider provideSpecialNamespaces + * @covers NamespaceInfo::getTalk + * @covers NamespaceInfo::isMethodValidFor + * + * @param int $ns + */ + public function testGetTalk_special( $ns ) { + $this->setExpectedException( MWException::class, + "NamespaceInfo::getTalk does not make any sense for given namespace $ns" ); + $this->newObj()->getTalk( $ns ); + } + + /** + * @dataProvider provideSpecialNamespaces + * @covers NamespaceInfo::getTalk + * @covers NamespaceInfo::getTalkPage + * @covers NamespaceInfo::isMethodValidFor + * + * @param int $ns + */ + public function testGetTalkPage_special( $ns ) { + $this->setExpectedException( MWException::class, + "NamespaceInfo::getTalk does not make any sense for given namespace $ns" ); + $this->newObj()->getTalkPage( new TitleValue( $ns, 'A' ) ); + } + + /** + * @dataProvider provideSpecialNamespaces + * @covers NamespaceInfo::getTalk + * @covers NamespaceInfo::getTalkPage + * @covers NamespaceInfo::isMethodValidFor + * @covers Title::getTalkPage + * + * @param int $ns + */ + public function testTitleGetTalkPage_special( $ns ) { + $this->setExpectedException( MWException::class, + "NamespaceInfo::getTalk does not make any sense for given namespace $ns" ); + Title::makeTitle( $ns, 'A' )->getTalkPage(); + } + + /** + * @dataProvider provideSpecialNamespaces + * @covers NamespaceInfo::getAssociated + * @covers NamespaceInfo::isMethodValidFor + * + * @param int $ns + */ + public function testGetAssociated_special( $ns ) { + $this->setExpectedException( MWException::class, + "NamespaceInfo::getAssociated does not make any sense for given namespace $ns" ); + $this->newObj()->getAssociated( $ns ); + } + + /** + * @dataProvider provideSpecialNamespaces + * @covers NamespaceInfo::getAssociated + * @covers NamespaceInfo::getAssociatedPage + * @covers NamespaceInfo::isMethodValidFor + * + * @param int $ns + */ + public function testGetAssociatedPage_special( $ns ) { + $this->setExpectedException( MWException::class, + "NamespaceInfo::getAssociated does not make any sense for given namespace $ns" ); + $this->newObj()->getAssociatedPage( new TitleValue( $ns, 'A' ) ); + } + + /** + * @dataProvider provideSpecialNamespaces + * @covers NamespaceInfo::getAssociated + * @covers NamespaceInfo::getAssociatedPage + * @covers NamespaceInfo::isMethodValidFor + * @covers Title::getOtherPage + * + * @param int $ns + */ + public function testTitleGetOtherPage_special( $ns ) { + $this->setExpectedException( MWException::class, + "NamespaceInfo::getAssociated does not make any sense for given namespace $ns" ); + Title::makeTitle( $ns, 'A' )->getOtherPage(); + } + + /** + * @dataProvider provideSubjectTalk + * @covers NamespaceInfo::getAssociated + * @covers NamespaceInfo::getAssociatedPage + * @covers Title::getOtherPage + * + * @param int $subject + * @param int $talk + */ + public function testGetAssociated( $subject, $talk ) { + $obj = $this->newObj(); + $this->assertSame( $talk, $obj->getAssociated( $subject ) ); + $this->assertSame( $subject, $obj->getAssociated( $talk ) ); + + $subjectTitle = new TitleValue( $subject, 'A' ); + $talkTitle = new TitleValue( $talk, 'A' ); + // Object will not be the same + $this->assertEquals( $talkTitle, $obj->getAssociatedPage( $subjectTitle ) ); + $this->assertEquals( $subjectTitle, $obj->getAssociatedPage( $talkTitle ) ); + + $subjectTitle = Title::makeTitle( $subject, 'A' ); + $talkTitle = Title::makeTitle( $talk, 'A' ); + $this->assertEquals( $talkTitle, $subjectTitle->getOtherPage() ); + $this->assertEquals( $subjectTitle, $talkTitle->getOtherPage() ); + } + + public static function provideSubjectTalk() { + return [ + // Format: [ subject, talk ] + 'Main/talk' => [ NS_MAIN, NS_TALK ], + 'User/user talk' => [ NS_USER, NS_USER_TALK ], + 'Unknown namespaces also supported' => [ 106, 107 ], + ]; + } + + public static function provideSpecialNamespaces() { + return [ + 'Special' => [ NS_SPECIAL ], + 'Media' => [ NS_MEDIA ], + 'Unknown negative index' => [ -613 ], + ]; + } + + // %} End getSubject/Talk/Associated + /********************************************************************************************** * Canonical namespaces * %{ -- 2.20.1