Make WikiPage a (non-proper) PageIdentity

This is a step towards introducing PageRecord.

We allow WikiPage to be a "non-proper" PageIdentity for now,
but this should be changed to a ProperPageIdentity as soon as
possible.

Bug: T272424
Change-Id: I194c55ec757e655117bccfeb7d6f5d8487b559e5
Signed-off-by: daniel <dkinzler@wikimedia.org>

RELEASE-NOTES-1.36

@@ -364,6 +364,12 @@ because of Phabricator reports.
   been added.  The methods were unused outside of MediaWiki core.
 * SquidPurgeClient and SquidPurgeClientPool, deprecated since 1.35, have been
   removed.
+* Several methods on WikiPage will now throw an exception when called on a
+  WikiPage instance that where constructed on a title that does not refer to a
+  proper page (but rather a special page or interwiki link). The behavior was
+  previously undefined and could in some cases lead to data corruption.
+  Affected methods are: getId(), insertOn(), newPageUpdater(),
+  doUpdateRestrictions(), doDeleteArticleReal(), doRollback(), doEditContent().
 * ParserTestRunner no longer invokes the ParserTestTables hook, instead
   cloning all database tables before running tests as MediaWikiIntegrationTest
   does.  If an extension was misusing the hook to *exclude* tables from the
@@ -502,6 +508,9 @@ because of Phabricator reports.
 * The InterwikiLoadPrefix hook has been deprecated; it is not compatible
   with future wikitext parsers (which need to enumerate all interwiki
   prefixes).  In test cases please use $wgInterwikiCache instead.
+* WikiPage instances should no longer be constructed for titles that do not
+  represent editable pages (e.g. special pages). WikiPages were always
+  documented to represent "MediaWiki article and history".
 * Skin::getSkinStylePath() has been deprecated. Please replace usages with
   the direct path to the resources.
 * The second argument of EnhancedChangesList::getDiffHistLinks, $query, has

includes/page/WikiPage.php

@@ -23,11 +23,13 @@
 use MediaWiki\Config\ServiceOptions;
 use MediaWiki\Content\ContentHandlerFactory;
 use MediaWiki\Content\IContentHandlerFactory;
+use MediaWiki\DAO\WikiAwareEntityTrait;
 use MediaWiki\Debug\DeprecatablePropertyArray;
 use MediaWiki\Edit\PreparedEdit;
 use MediaWiki\HookContainer\ProtectedHookAccessorTrait;
 use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
+use MediaWiki\Page\PageIdentity;
 use MediaWiki\Page\ParserOutputAccess;
 use MediaWiki\Revision\RevisionRecord;
 use MediaWiki\Revision\RevisionRenderer;
@@ -40,6 +42,7 @@ use MediaWiki\Storage\EditResultCache;
 use MediaWiki\Storage\PageUpdater;
 use MediaWiki\Storage\RevisionSlotsUpdate;
 use Wikimedia\Assert\Assert;
+use Wikimedia\Assert\PreconditionException;
 use Wikimedia\IPUtils;
 use Wikimedia\NonSerializable\NonSerializableTrait;
 use Wikimedia\Rdbms\FakeResultWrapper;
@@ -52,9 +55,10 @@ use Wikimedia\Rdbms\LoadBalancer;
  * Some fields are public only for backwards-compatibility. Use accessors.
  * In the past, this class was part of Article.php and everything was public.
  */
-class WikiPage implements Page, IDBAccessObject {
+class WikiPage implements Page, IDBAccessObject, PageIdentity {
 	use NonSerializableTrait;
 	use ProtectedHookAccessorTrait;
+	use WikiAwareEntityTrait;
 
 	// Constants for $mDataLoadedFrom and related
 
@@ -134,9 +138,13 @@ class WikiPage implements Page, IDBAccessObject {
 	private $derivedDataUpdater = null;
 
 	/**
-	 * @param Title $title
+	 * @param PageIdentity $pageIdentity
 	 */
-	public function __construct( Title $title ) {
+	public function __construct( PageIdentity $pageIdentity ) {
+		$pageIdentity->assertWiki( PageIdentity::LOCAL );
+
+		// TODO: remove the need for casting to Title.
+		$title = Title::castFromPageIdentity( $pageIdentity );
 		if ( !$title->canExist() ) {
 			// TODO: In order to allow WikiPage to implement ProperPageIdentity,
 			//       throw here to prevent construction of a WikiPage that doesn't
@@ -159,16 +167,18 @@ class WikiPage implements Page, IDBAccessObject {
 	}
 
 	/**
-	 * Create a WikiPage object of the appropriate class for the given title.
+	 * Create a WikiPage object of the appropriate class for the given PageIdentity.
+	 * The PageIdentity must represent a proper page that can exist on the wiki,
+	 * that is, not a special page or media link or section link or interwiki link.
 	 *
-	 * @param Title $title
+	 * @param PageIdentity $pageIdentity
 	 *
 	 * @throws MWException
 	 * @return WikiPage|WikiCategoryPage|WikiFilePage
 	 * @deprecated since 1.36, use WikiPageFactory::newFromTitle instead
 	 */
-	public static function factory( Title $title ) {
+	public static function factory( PageIdentity $pageIdentity ) {
-		return MediaWikiServices::getInstance()->getWikiPageFactory()->newFromTitle( $title );
+		return MediaWikiServices::getInstance()->getWikiPageFactory()->newFromTitle( $pageIdentity );
 	}
 
 	/**
@@ -410,6 +420,10 @@ class WikiPage implements Page, IDBAccessObject {
 	 * @return stdClass|false Database result resource, or false on failure
 	 */
 	public function pageDataFromTitle( $dbr, $title, $options = [] ) {
+		if ( !$title->canExist() ) {
+			return false;
+		}
+
 		return $this->pageData( $dbr, [
 			'page_namespace' => $title->getNamespace(),
 			'page_title' => $title->getDBkey() ], $options );
@@ -562,9 +576,32 @@ class WikiPage implements Page, IDBAccessObject {
 	}
 
 	/**
+	 * Code that requires this WikiPage to be a "proper page" in the sense
+	 * defined by PageIdentity should call this method.
+	 *
+	 * @note In the future, this method should become redundant, as the
+	 * constructor should not allow a WikiPage to be constructed for as title
+	 * that does not represent a proper page. For the time being, we allow
+	 * such instances for backwards compatibility.
+	 *
+	 * @throws PreconditionException
+	 */
+	private function assertProperPage() {
+		Assert::precondition(
+			$this->mTitle->canExist(),
+			'This WikiPage instance does not represent a proper page!'
+		);
+	}
+
+	/**
+	 * @param string|false $wikiId
+	 *
 	 * @return int Page ID
 	 */
-	public function getId() {
+	public function getId( $wikiId = self::LOCAL ): int {
+		$this->assertWiki( $wikiId );
+		$this->assertProperPage();
+
 		if ( !$this->mDataLoaded ) {
 			$this->loadPageData();
 		}
@@ -574,7 +611,7 @@ class WikiPage implements Page, IDBAccessObject {
 	/**
 	 * @return bool Whether or not the page exists in the database
 	 */
-	public function exists() {
+	public function exists(): bool {
 		if ( !$this->mDataLoaded ) {
 			$this->loadPageData();
 		}
@@ -1313,6 +1350,8 @@ class WikiPage implements Page, IDBAccessObject {
 	 *   page ID is already in use.
 	 */
 	public function insertOn( $dbw, $pageId = null ) {
+		$this->assertProperPage();
+
 		$pageIdForInsert = $pageId ? [ 'page_id' => $pageId ] : [];
 		$dbw->insert(
 			'page',
@@ -1820,6 +1859,8 @@ class WikiPage implements Page, IDBAccessObject {
 	 * @return PageUpdater
 	 */
 	public function newPageUpdater( User $user, RevisionSlotsUpdate $forUpdate = null ) {
+		$this->assertProperPage();
+
 		$config = MediaWikiServices::getInstance()->getMainConfig();
 
 		$pageUpdater = new PageUpdater(
@@ -2324,6 +2365,8 @@ class WikiPage implements Page, IDBAccessObject {
 	) {
 		global $wgCascadingRestrictionLevels;
 
+		$this->assertProperPage();
+
 		if ( wfReadOnly() ) {
 			return Status::newFatal( wfMessage( 'readonlytext', wfReadOnlyReason() ) );
 		}
@@ -2785,6 +2828,7 @@ class WikiPage implements Page, IDBAccessObject {
 		$tags = [], $logsubtype = 'delete', $immediate = false
 	) {
 		wfDebug( __METHOD__ );
+		$this->assertProperPage();
 
 		$status = Status::newGood();
 
@@ -3227,6 +3271,8 @@ class WikiPage implements Page, IDBAccessObject {
 	public function doRollback(
 		$fromP, $summary, $token, $bot, &$resultDetails, User $user, $tags = null
 	) {
+		$this->assertProperPage();
+
 		$resultDetails = null;
 
 		// Check permissions
@@ -4070,4 +4116,70 @@ class WikiPage implements Page, IDBAccessObject {
 		$this->clear();
 	}
 
+	/**
+	 * @inheritDoc
+	 * @since 1.36
+	 */
+	public function getNamespace(): int {
+		return $this->getTitle()->getNamespace();
+	}
+
+	/**
+	 * @inheritDoc
+	 * @since 1.36
+	 */
+	public function getDBkey(): string {
+		return $this->getTitle()->getDBkey();
+	}
+
+	/**
+	 * @return false self::LOCAL
+	 * @since 1.36
+	 */
+	public function getWikiId() {
+		return $this->getTitle()->getWikiId();
+	}
+
+	/**
+	 * @return true
+	 * @since 1.36
+	 */
+	public function canExist(): bool {
+		// NOTE: once WikiPage becomes a ProperPageIdentity, this should always return true
+		return $this->mTitle->canExist();
+	}
+
+	/**
+	 * @inheritDoc
+	 * @since 1.36
+	 */
+	public function __toString(): string {
+		return $this->mTitle->__toString();
+	}
+
+	/**
+	 * @inheritDoc
+	 * @since 1.36
+	 *
+	 * @param PageIdentity $other
+	 * @return bool
+	 */
+	public function isSamePageAs( PageIdentity $other ): bool {
+		// NOTE: keep in sync with PageIdentityValue::isSamePageAs()!
+
+		if ( $other->getWikiId() !== $this->getWikiId()
+			|| $other->getId() !== $this->getId() ) {
+			return false;
+		}
+
+		if ( $this->getId() === 0 ) {
+			if ( $other->getNamespace() !== $this->getNamespace()
+				|| $other->getDBkey() !== $this->getDBkey() ) {
+				return false;
+			}
+		}
+
+		return true;
+	}
+
 }

includes/page/WikiPageFactory.php

@@ -6,6 +6,7 @@ use DBAccessObjectUtils;
 use InvalidArgumentException;
 use MediaWiki\Linker\LinkTarget;
 use MediaWiki\Page\Hook\WikiPageFactoryHook;
+use stdClass;
 use Title;
 use TitleFactory;
 use WikiCategoryPage;
@@ -45,19 +46,27 @@ class WikiPageFactory {
 	/**
 	 * Create a WikiPage object from a title.
 	 *
-	 * @param Title $title
+	 * @param PageIdentity $pageIdentity
 	 *
 	 * @return WikiPage
 	 */
-	public function newFromTitle( Title $title ) {
+	public function newFromTitle( PageIdentity $pageIdentity ) {
-		$ns = $title->getNamespace();
+		if ( $pageIdentity instanceof WikiPage ) {
-
+			return $pageIdentity;
-		if ( $ns == NS_MEDIA ) {
-			throw new InvalidArgumentException( "NS_MEDIA is a virtual namespace; use NS_FILE." );
-		} elseif ( $ns < 0 ) {
-			throw new InvalidArgumentException( "Invalid or virtual namespace $ns given." );
 		}
 
+		if ( !$pageIdentity->canExist() ) {
+			throw new InvalidArgumentException(
+				"The given PageIdentity does not represent a proper page"
+			);
+		}
+
+		$ns = $pageIdentity->getNamespace();
+
+		// TODO: remove the need for casting to Title. We'll have to create a new hook to
+		//       replace the WikiPageFactory hook.
+		$title = Title::castFromPageIdentity( $pageIdentity );
+
 		$page = null;
 		if ( !$this->wikiPageFactoryHookRunner->onWikiPageFactory( $title, $page ) ) {
 			return $page;
@@ -91,7 +100,7 @@ class WikiPageFactory {
 	/**
 	 * Create a WikiPage object from a database row
 	 *
-	 * @param \stdClass $row Database row containing at least fields returned by getQueryInfo().
+	 * @param stdClass $row Database row containing at least fields returned by getQueryInfo().
 	 * @param string|int $from Source of $data:
 	 *        - "fromdb" or WikiPage::READ_NORMAL: from a replica DB
 	 *        - "fromdbmaster" or WikiPage::READ_LATEST: from the master DB

tests/phpunit/includes/content/TextContentTest.php

@@ -379,7 +379,7 @@ class TextContentTest extends MediaWikiLangTestCase {
 	 * @covers TextContent::getDeletionUpdates
 	 */
 	public function testDeletionUpdates( $model, $text, $expectedStuff ) {
-		$page = $this->getNonexistingTestPage( __METHOD__ );
+		$page = $this->getNonexistingTestPage( get_class( $this ) . '::' . __FUNCTION__ );
 		$title = $page->getTitle();
 
 		$content = ContentHandler::makeContent( $text, $title, $model );

tests/phpunit/includes/page/WikiPageDbTest.php

@@ -2,6 +2,8 @@
 
 use MediaWiki\Edit\PreparedEdit;
 use MediaWiki\MediaWikiServices;
+use MediaWiki\Page\PageIdentity;
+use MediaWiki\Page\PageIdentityValue;
 use MediaWiki\Revision\MutableRevisionRecord;
 use MediaWiki\Revision\RevisionRecord;
 use MediaWiki\Revision\SlotRecord;
@@ -60,7 +62,7 @@ class WikiPageDbTest extends MediaWikiLangTestCase {
 			/** @var WikiPage $p */
 
 			try {
-				if ( $p->exists() ) {
+				if ( $p->canExist() && $p->exists() ) {
 					$p->doDeleteArticleReal( "testing done.", $user );
 				}
 			} catch ( MWException $ex ) {
@@ -149,11 +151,12 @@ class WikiPageDbTest extends MediaWikiLangTestCase {
 		$this->filterDeprecated( '/Accessing WikiPage that cannot exist as a page/' );
 		$page = new WikiPage( $title );
 
+		$this->assertFalse( $page->canExist() );
 		$this->assertFalse( $page->exists() );
 		$this->assertNull( $page->getRevisionRecord() );
 
-		// NOTE: once WikiPage becomes a PageIdentity, getId() should throw!
+		$this->expectException( RuntimeException::class );
-		$this->assertSame( 0, $page->getId() );
+		$page->getId();
 	}
 
 	/**
@@ -328,7 +331,8 @@ class WikiPageDbTest extends MediaWikiLangTestCase {
 
 		global $wgUser;
 
-		$page = $this->newPage( __METHOD__ );
+		// NOTE: Test that Editing also works with a fragment title!
+		$page = $this->newPage( __METHOD__ . '#Fragment' );
 		$title = $page->getTitle();
 
 		$content = ContentHandler::makeContent(
@@ -535,9 +539,10 @@ class WikiPageDbTest extends MediaWikiLangTestCase {
 			CONTENT_MODEL_WIKITEXT
 		);
 
+		$this->filterDeprecated( '/WikiPage constructed on a Title that cannot exist as a page/' );
 		try {
 			$page = $this->newPage( $title );
-			$status = $page->doUserEditContent( $content, $user1, "[[testing]] 1", EDIT_NEW );
+			$page->doUserEditContent( $content, $user1, "[[testing]] 1", EDIT_NEW );
 		} catch ( Exception $ex ) {
 			// Throwing is an acceptable way to react to an invalid title,
 			// as long as no garbage is written to the database.
@@ -1720,6 +1725,15 @@ more stuff
 		$title = Title::makeTitle( NS_CATEGORY, 'SomeCategory' );
 		$page = WikiPage::factory( $title );
 		$this->assertEquals( WikiCategoryPage::class, get_class( $page ) );
+
+		$title = Title::makeTitle( NS_MAIN, 'SomePage' );
+		$page = WikiPage::factory( $title );
+		$this->assertEquals( WikiPage::class, get_class( $page ) );
+		$this->assertSame( $page, WikiPage::factory( $page ) );
+
+		$title = new PageIdentityValue( 0, NS_MAIN, 'SomePage', PageIdentity::LOCAL );
+		$page = WikiPage::factory( $title );
+		$this->assertEquals( WikiPage::class, get_class( $page ) );
 	}
 
 	/**