Move Title::getSubject/Talk/OtherPage to NamespaceInfo
authorAryeh Gregor <ayg@aryeh.name>
Mon, 29 Apr 2019 16:29:31 +0000 (19:29 +0300)
committerJames D. Forrester <jforrester@wikimedia.org>
Mon, 6 May 2019 15:29:28 +0000 (08:29 -0700)
This allows converting some more code to LinkTarget. 100% test coverage.

Change-Id: I28903af6a41d02755f37f31561a524547445821e

RELEASE-NOTES-1.34
includes/Title.php
includes/title/NamespaceInfo.php
tests/phpunit/includes/TitleTest.php
tests/phpunit/includes/title/NamespaceInfoTest.php

index 98ed961..b58c269 100644 (file)
@@ -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 ===
 * …
index 4cac844..dc5400e 100644 (file)
@@ -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 ) );
        }
 
        /**
index e3ab3a3..7cfadc0 100644 (file)
@@ -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
         *
index 2159a35..c46f69b 100644 (file)
@@ -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
index 5e67636..556c640 100644 (file)
@@ -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
         * %{