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>
This commit is contained in:
parent
4478047d6e
commit
65f89072e3
5 changed files with 168 additions and 24 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 ) {
|
||||
return MediaWikiServices::getInstance()->getWikiPageFactory()->newFromTitle( $title );
|
||||
public static function factory( PageIdentity $pageIdentity ) {
|
||||
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;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 ) {
|
||||
$ns = $title->getNamespace();
|
||||
|
||||
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." );
|
||||
public function newFromTitle( PageIdentity $pageIdentity ) {
|
||||
if ( $pageIdentity instanceof WikiPage ) {
|
||||
return $pageIdentity;
|
||||
}
|
||||
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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 );
|
||||
|
|
|
|||
|
|
@ -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->assertSame( 0, $page->getId() );
|
||||
$this->expectException( RuntimeException::class );
|
||||
$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 ) );
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
Loading…
Reference in a new issue